[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-19 Thread Lex Spoon 

On Tue, Jun 9, 2009 at 11:01 AM, Matt Mastraccimatt...@mastracci.com wrote:
 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.

I expect this would work well, too.


 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.

To be precise, I think what you're getting at is that within-method
data flow would be helpful to represent.  It's safer to stay with an
AST representation due to the fact that the GWT compiler must output
valid JavaScript syntax, and in particular does not have the luxury of
using any kind of goto.  However, it could certainly use an AST but
either have the AST follow the SSA invariant, or else augment the AST
with a data-flow graph.  Either way, the goal is to get data flow
information in there, so that silly code like (x=10,x) can be
replaced by simply 10.

-Lex

--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-10 Thread scottb 

LGTM.  Want to point out that we can optimize some of these even better
in cases where the nested code is an expression statement.


http://gwt-code-reviews.appspot.com/33845/diff/1/2
File dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java (right):

http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode37
Line 37: assertEquals(if(!a()){b()}, optimize(if (a()) { } else {
b(); }));
a()||b()

http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode45
Line 45: assertEquals(if(a()){b()}, optimize(if (a()) { b() } else {
}));
a()b()

http://gwt-code-reviews.appspot.com/33845

--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-10 Thread Matt Mastracci 
SGTM.  I'll start a new review with the remainder of the changes.

On 10-Jun-09, at 2:16 PM, Scott Blum wrote:

 I thought I'd go ahead and commit what you have so far, if you don't  
 object?

 On Wed, Jun 10, 2009 at 4:12 PM, mmastrac.altern...@gmail.com wrote:
 I'll update the patch with this change.  While I'm at it, I may as  
 well
 tackle this one too:

 if (test) {a()} else {b()} - test?a():b()


 On 2009/06/10 19:56:18, scottb wrote:
 LGTM.  Want to point out that we can optimize some of these even
 better in cases
 where the nested code is an expression statement.

 http://gwt-code-reviews.appspot.com/33845/diff/1/2
 File dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java
 (right):

 http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode37
 Line 37: assertEquals(if(!a()){b()}, optimize(if (a()) { } else {
 b(); }));
 a()||b()

 http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode45
 Line 45: assertEquals(if(a()){b()}, optimize(if (a()) { b() } else
 { }));
 a()b()



 http://gwt-code-reviews.appspot.com/33845



--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-10 Thread Ray Cromwell 

Scott, do you mean replacing

if(a != null) { expr; }

with

a  expr?

-Ray


On Thu, Jun 11, 2009 at 4:56 AM, sco...@google.com wrote:

 LGTM.  Want to point out that we can optimize some of these even better
 in cases where the nested code is an expression statement.


 http://gwt-code-reviews.appspot.com/33845/diff/1/2
 File dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java (right):

 http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode37
 Line 37: assertEquals(if(!a()){b()}, optimize(if (a()) { } else {
 b(); }));
 a()||b()

 http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode45
 Line 45: assertEquals(if(a()){b()}, optimize(if (a()) { b() } else {
 }));
 a()b()

 http://gwt-code-reviews.appspot.com/33845

 


--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-10 Thread Scott Blum 
Ultimately that'd be great, whether that's a one step operation or a
two-step.

On Wed, Jun 10, 2009 at 7:16 PM, Ray Cromwell cromwell...@gmail.com wrote:


 Scott, do you mean replacing

 if(a != null) { expr; }

 with

 a  expr?

 -Ray


 On Thu, Jun 11, 2009 at 4:56 AM, sco...@google.com wrote:
 
  LGTM.  Want to point out that we can optimize some of these even better
  in cases where the nested code is an expression statement.
 
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2
  File dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java (right):
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode37
  Line 37: assertEquals(if(!a()){b()}, optimize(if (a()) { } else {
  b(); }));
  a()||b()
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode45
  Line 45: assertEquals(if(a()){b()}, optimize(if (a()) { b() } else {
  }));
  a()b()
 
  http://gwt-code-reviews.appspot.com/33845
 
  
 

 


--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-10 Thread Matt Mastracci 

After I sent that message, I peeked into EqualityNormalizer which is  
basically pre-munging all the equality operators that compare against  
null to Cast.isNull() and Cast.isNotNull() as I described below.

On 10-Jun-09, at 8:16 PM, Matt Mastracci wrote:

 That construct isn't a generically safe JS optimization, unless you  
 have additional type info at the JS level to tell you what Java type  
 the ifExpr was.  I don't know for sure whether that information is  
 available after the transition to JS (does SOYC make this available?).

 I believe that GenerateJavaScriptAST could be a bit smarter and not  
 generate == null and != null for binary equality ops where the  
 operand is known to be a reference type that isn't a string.

 One example failure case where null equality is not equivalent to  
 binary coersion is the following:

 var a = false;
 if (a != null) { alert('blah'); }

 and

 var a = false;
 a  alert('blah')


 BTW, this quick test shows that the problematic values are 0  
 (numeric zero),  (the empty string), NaN and false:

 table
 script
 tests = [ [], {}, 0, null, undefined, , 0, null, undefined,  
 NaN, NaN, true, false ]
 for (i = 0; i  tests.length; i++) {
  document.write(trtd + ( + tests[i]) + /tdtd + (! 
 tests[i]) + /tdtd + (tests[i] == null) + /td/tr);
 }
 /script


 On 10-Jun-09, at 5:20 PM, Scott Blum wrote:

 Ultimately that'd be great, whether that's a one step operation or  
 a two-step.

 On Wed, Jun 10, 2009 at 7:16 PM, Ray Cromwell  
 cromwell...@gmail.com wrote:

 Scott, do you mean replacing

 if(a != null) { expr; }

 with

 a  expr?

 -Ray


 On Thu, Jun 11, 2009 at 4:56 AM, sco...@google.com wrote:
 
  LGTM.  Want to point out that we can optimize some of these even  
 better
  in cases where the nested code is an expression statement.
 
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2
  File dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java  
 (right):
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode37
  Line 37: assertEquals(if(!a()){b()}, optimize(if (a()) { }  
 else {
  b(); }));
  a()||b()
 
  http://gwt-code-reviews.appspot.com/33845/diff/1/2#newcode45
  Line 45: assertEquals(if(a()){b()}, optimize(if (a()) { b() }  
 else {
  }));
  a()b()
 
  http://gwt-code-reviews.appspot.com/33845
 
  
 

 




--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-09 Thread Ray Cromwell 

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, mmastrac.altern...@gmail.com 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,
 +           *

[gwt-contrib] Re: More if-statement optimizations in JsStaticEval

2009-06-09 Thread Matt Mastracci 

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, mmastrac.altern...@gmail.com 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