Yeah... I would love to have a single AST to represent both of the language states - many optimizations don't happen because we toss all the Java type info during the conversion to JS.
For instance, you can't safely optimize this: if (blah != null) to this: if (blah) without knowing what type blah was at the beginning. In addition, a lot of optimizations would be a lot simpler to write (and some would just "fall out" of that structure) if we were dealing with a more abstract SSA version of the code, rather than a more direct AST. On 9-Jun-09, at 5:40 AM, Ray Cromwell wrote: > > This brings up the issue of whether or not there isn't someway to > abstract some of these optimizations on the ASTs so that they can be > shared by the Java and JS optimizers. I made patch along the same > lines to the simplifier mirroring the Java simplifier. It seems like a > lot of optimizations are being duplicated. > > -Ray > > > On Tue, Jun 9, 2009 at 1:12 AM, <[email protected]> wrote: >> >> Reviewers: scottb, >> >> Description: >> I'm tackling another small issue on the JS side to get myself up to >> speed with the code (esp. the SOYC changes throughout) and help shake >> out all the cobwebs since my last set of patches :). >> >> This is a simple patch to improve the optimization of if statements. >> These items show up in compiled output, mostly in the generated >> exception handler code. >> >> It adds two extra optimizations: >> >> - Empty, but present else statements are pruned: >> >> if (x) {doSomething();} else {} -> if (x) {doSomething();} >> >> - If statements with empty "then" conditions are inverted: >> >> if (x) {} else {doSomething()} -> if (!x) {doSomething();} >> >> I also cleaned up my previous set of unit tests to use the clearer >> assertEquals() instead of assertTrue(x.equals(...)). >> >> >> Please review this at http://gwt-code-reviews.appspot.com/33845 >> >> Affected files: >> dev/core/src/com/google/gwt/dev/js/JsStaticEval.java >> dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java >> >> >> Index: dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java >> =================================================================== >> --- dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java >> (revision >> 5519) >> +++ dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java >> (working copy) >> @@ -29,20 +29,36 @@ >> >> public class JsStaticEvalTest extends TestCase { >> >> + public void testIfWithEmptyThen() throws Exception { >> + assertEquals("a();", optimize("if (a()) { }")); >> + } >> + >> + public void testIfWithEmptyThenAndElse() throws Exception { >> + assertEquals("if(!a()){b()}", optimize("if (a()) { } else >> { b(); }")); >> + } >> + >> + public void testIfWithEmptyThenAndEmptyElse() throws Exception { >> + assertEquals("a();", optimize("if (a()) { } else { }")); >> + } >> + >> + public void testIfWithThenAndEmptyElse() throws Exception { >> + assertEquals("if(a()){b()}", optimize("if (a()) { b() } else >> { }")); >> + } >> + >> public void testLiteralEqNull() throws Exception { >> - assertTrue(optimize("alert('test' == >> null)").equals("alert(false);")); >> + assertEquals("alert(false);", optimize("alert('test' == >> null)")); >> } >> >> public void testLiteralNeNull() throws Exception { >> - assertTrue(optimize("alert('test' != >> null)").equals("alert(true);")); >> + assertEquals("alert(true);", optimize("alert('test' != null)")); >> } >> >> public void testNullEqNull() throws Exception { >> - assertTrue(optimize("alert(null == >> null)").equals("alert(true);")); >> + assertEquals("alert(true);", optimize("alert(null == null)")); >> } >> >> public void testNullNeNull() throws Exception { >> - assertTrue(optimize("alert(null != >> null)").equals("alert(false);")); >> + assertEquals("alert(false);", optimize("alert(null != null)")); >> } >> >> private String optimize(String js) throws Exception { >> Index: dev/core/src/com/google/gwt/dev/js/JsStaticEval.java >> =================================================================== >> --- dev/core/src/com/google/gwt/dev/js/JsStaticEval.java >> (revision 5519) >> +++ dev/core/src/com/google/gwt/dev/js/JsStaticEval.java >> (working copy) >> @@ -35,6 +35,7 @@ >> import com.google.gwt.dev.js.ast.JsPrefixOperation; >> import com.google.gwt.dev.js.ast.JsProgram; >> import com.google.gwt.dev.js.ast.JsStatement; >> +import com.google.gwt.dev.js.ast.JsUnaryOperation; >> import com.google.gwt.dev.js.ast.JsUnaryOperator; >> import com.google.gwt.dev.js.ast.JsValueLiteral; >> import com.google.gwt.dev.js.ast.JsVars; >> @@ -348,8 +349,37 @@ >> block.getStatements().add(decls); >> } >> ctx.replaceMe(accept(block)); >> - } else if (isEmpty(thenStmt) && isEmpty(elseStmt)) { >> - ctx.replaceMe(expr.makeStmt()); >> + } else { >> + boolean thenIsEmpty = isEmpty(thenStmt); >> + boolean elseIsEmpty = isEmpty(elseStmt); >> + >> + if (thenIsEmpty && elseIsEmpty) { >> + ctx.replaceMe(expr.makeStmt()); >> + } else if (thenIsEmpty && !elseIsEmpty) { >> + /* >> + * If the then block is blank, but the else statement has >> statements, >> + * invert the test >> + */ >> + sourceInfo = >> x.getSourceInfo().makeChild(StaticEvalVisitor.class, >> + "Simplified if with empty then statement"); >> + >> + JsUnaryOperation negatedOperation = new >> JsPrefixOperation(sourceInfo, >> + JsUnaryOperator.NOT, x.getIfExpr()); >> + JsIf newIf = new JsIf(sourceInfo, negatedOperation, >> elseStmt, >> null); >> + >> + ctx.replaceMe(accept(newIf)); >> + } else if (elseIsEmpty && elseStmt != null) { >> + /* >> + * If the else statement is present but has no effective >> statements, >> + * prune it >> + */ >> + sourceInfo = >> x.getSourceInfo().makeChild(StaticEvalVisitor.class, >> + "Pruned empty else statement"); >> + >> + JsIf newIf = new JsIf(sourceInfo, x.getIfExpr(), >> thenStmt, null); >> + >> + ctx.replaceMe(accept(newIf)); >> + } >> } >> } >> >> >> >> >>> >> > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
