dart2js cps: Disable RHS propagation in operands to builtin operators.

pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart

Index: pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
diff --git a/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart b/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
index 32b508ff2cc92ed7615804b77b4ba6fc98597151..526e5588784b806d66925b99f36d810e6162ba34 100644
--- a/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
+++ b/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
@@ -167,7 +167,7 @@ class StatementRewriter extends Transformer implements Pass {
 
   /// Binding environment for variables that are assigned to effectively
   /// constant expressions (see [isEffectivelyConstant]).
-  Map<Variable, Expression> constantEnvironment;
+  Map<Variable, Expression> constantEnvironment = <Variable, Expression>{};
 
   /// Substitution map for labels. Any break to a label L should be substituted
   /// for a break to L' if L maps to L'.
@@ -186,14 +186,6 @@ class StatementRewriter extends Transformer implements Pass {
   /// variable might have changed since it was put in the environment.
   final Map<Variable, int> dominatingAssignments = <Variable, int>{};
 
-  /// Rewriter for methods.
-  StatementRewriter() : constantEnvironment = <Variable, Expression>{};
-
-  /// Rewriter for nested functions.
-  StatementRewriter.nested(StatementRewriter parent)
-      : constantEnvironment = parent.constantEnvironment,
-        unseenUses = parent.unseenUses;
-
   /// A set of labels that can be safely inlined at their use.
   ///
   /// The successor statements for labeled statements that have only one break
@@ -202,6 +194,13 @@ class StatementRewriter extends Transformer implements Pass {
   /// handler (i.e., if the code would be moved into a try from outside it).
   Set<Label> safeForInlining = new Set<Label>();
 
+  /// If the top element is true, assignments of form "x = CONST" may be
+  /// propagated into a following occurence of CONST.  This may confuse the JS
+  /// engine so it is disabled in some cases.
+  final List<bool> allowRhsPropagation = <bool>[true];
+
+  bool get isRhsPropagationAllowed => allowRhsPropagation.last;
+
   /// Returns the redirect target of [jump] or [jump] itself if it should not
   /// be redirected.
   Jump redirect(Jump jump) {
@@ -327,7 +326,8 @@ class StatementRewriter extends Transformer implements Pass {
       //
       //     { E.foo = x; bar(x) } ==> bar(E.foo = x)
       //
-      if (getRightHandVariable(binding) == node.variable) {
+      if (isRhsPropagationAllowed &&
+          getRightHandVariable(binding) == node.variable) {
         environment.removeLast();
         --node.variable.readCount;
         return visitExpression(binding);
@@ -467,7 +467,9 @@ class StatementRewriter extends Transformer implements Pass {
   }
 
   Expression visitAssign(Assign node) {
+    allowRhsPropagation.add(true);
     node.value = visitExpression(node.value);
+    allowRhsPropagation.removeLast();
     // Remove assignments to variables without any uses. This can happen
     // because the assignment was propagated into its use, e.g:
     //
@@ -482,10 +484,12 @@ class StatementRewriter extends Transformer implements Pass {
 
   /// Process nodes right-to-left, the opposite of evaluation order in the case
   /// of argument lists..
-  void _rewriteList(List<Node> nodes) {
+  void _rewriteList(List<Node> nodes, {bool rhsPropagation: true}) {
+    allowRhsPropagation.add(rhsPropagation);
     for (int i = nodes.length - 1; i >= 0; --i) {
       nodes[i] = visitExpression(nodes[i]);
     }
+    allowRhsPropagation.removeLast();
   }
 
   Expression visitInvokeStatic(InvokeStatic node) {
@@ -721,7 +725,7 @@ class StatementRewriter extends Transformer implements Pass {
   }
 
   Expression visitConstant(Constant node) {
-    if (!environment.isEmpty) {
+    if (isRhsPropagationAllowed && !environment.isEmpty) {
       Constant constant = getRightHandConstant(environment.last);
       if (constant != null && constant.value == node.value) {
         return visitExpression(environment.removeLast());
@@ -755,8 +759,10 @@ class StatementRewriter extends Transformer implements Pass {
   }
 
   Expression visitSetField(SetField node) {
+    allowRhsPropagation.add(true);
     node.value = visitExpression(node.value);
     node.object = visitExpression(node.object);
+    allowRhsPropagation.removeLast();
     return node;
   }
 
@@ -770,7 +776,9 @@ class StatementRewriter extends Transformer implements Pass {
   }
 
   Expression visitSetStatic(SetStatic node) {
+    allowRhsPropagation.add(true);
     node.value = visitExpression(node.value);
+    allowRhsPropagation.removeLast();
     return node;
   }
 
@@ -880,31 +888,31 @@ class StatementRewriter extends Transformer implements Pass {
   /// foo() must be evaluated before bar(), so the propagation is only possible
   /// by commuting the operator.
   Expression visitApplyBuiltinOperator(ApplyBuiltinOperator node) {
-    if (environment.isEmpty || getLeftHand(environment.last) == null) {
-      // If there is no recent assignment that might propagate, so there is no
-      // opportunity for optimization here.
-      _rewriteList(node.arguments);
-      return node;
-    }
-    Variable propagatableVariable = getLeftHand(environment.last);
-    BuiltinOperator commuted = commuteBinaryOperator(node.operator);
-    if (commuted != null) {
-      assert(node.arguments.length == 2); // Only binary operators can commute.
-      Expression left = node.arguments[0];
-      if (left is VariableUse && propagatableVariable == left.variable) {
-        Expression right = node.arguments[1];
-        if (right is This ||
-            (right is VariableUse &&
-             propagatableVariable != right.variable &&
-             !constantEnvironment.containsKey(right.variable))) {
-          // An assignment can be propagated if we commute the operator.
-          node.operator = commuted;
-          node.arguments[0] = right;
-          node.arguments[1] = left;
+    if (!environment.isEmpty && getLeftHand(environment.last) != null) {
+      Variable propagatableVariable = getLeftHand(environment.last);
+      BuiltinOperator commuted = commuteBinaryOperator(node.operator);
+      if (commuted != null) {
+        // Only binary operators can commute.
+        assert(node.arguments.length == 2);
+        Expression left = node.arguments[0];
+        if (left is VariableUse && propagatableVariable == left.variable) {
+          Expression right = node.arguments[1];
+          if (right is This ||
+              (right is VariableUse &&
+               propagatableVariable != right.variable &&
+               !constantEnvironment.containsKey(right.variable))) {
+            // An assignment can be propagated if we commute the operator.
+            node.operator = commuted;
+            node.arguments[0] = right;
+            node.arguments[1] = left;
+          }
         }
       }
     }
-    _rewriteList(node.arguments);
+    // Avoid code like `p == (q.f = null)`. JS operators with a constant operand
+    // can sometimes be compiled to a specialized instruction in the JS engine,
+    // so retain syntactically constant operands.
+    _rewriteList(node.arguments, rhsPropagation: false);
     return node;
   }