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

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

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library tree_ir.optimization.statement_rewriter;
 
 import 'optimization.dart' show Pass;
 import '../tree_ir_nodes.dart';
 import '../../io/source_information.dart';
 import '../../elements/elements.dart';
@@ skipping 149 matching lines @@
   /// Assignments with constant right-hand sides (see [isEffectivelyConstant])
   /// are not considered impure and are put in [constantEnvironment] instead.
   ///
   /// Except for [Conditional]s, expressions in the environment have
   /// not been processed, and all their subexpressions must therefore be
   /// variables uses.
   List<Expression> environment = <Expression>[];
 
   /// 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'.
   Map<Label, Jump> labelRedirects = <Label, Jump>{};
 
   /// Number of uses of the given variable that are still unseen.
   /// Used to detect the first use of a variable (since we do backwards
   /// traversal, the first use is the last one seen).
   Map<Variable, int> unseenUses = <Variable, int>{};
 
   /// Number of assignments to a given variable that dominate the current
   /// position.
   ///
   /// Pure expressions will not be inlined if it uses a variable with more than
   /// one dominating assignment, because the reaching definition of the used
   /// 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
   /// from them are normally rewritten inline at the site of the break.  This
   /// is not safe if the code would be moved inside the scope of an exception
   /// 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) {
     Jump newJump = labelRedirects[jump.target];
     return newJump != null ? newJump : jump;
   }
 
   void inEmptyEnvironment(void action(), {bool keepConstants: true}) {
     List oldEnvironment = environment;
     Map oldConstantEnvironment = constantEnvironment;
@@ skipping 105 matching lines @@
         return visitExpression(binding);
       }
 
       // Is the most recently evaluated impure expression known to have the
       // value of this variable?
       //
       // If so, we can replace this use with the impure expression, e.g:
       //
       //     { 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);
       }
     }
 
     // If the definition could not be propagated, leave the variable use.
     return node;
   }
 
@@ skipping 119 matching lines @@
           return stmt;
         } else {
           // Expression was propagated into the successor.
           return stmt.next;
         }
       };
     }
   }
 
   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:
     //
     //     { x = foo(); bar(x) } ==> bar(x = foo()) ==> bar(foo())
     //
     if (node.variable.readCount == 0) {
       --node.variable.writeCount;
       return node.value;
     }
     return node;
   }
 
   /// 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) {
     _rewriteList(node.arguments);
     return node;
   }
 
   Expression visitInvokeMethod(InvokeMethod node) {
     if (node.receiverIsNotNull) {
       _rewriteList(node.arguments);
@@ skipping 215 matching lines @@
       node.tryBody = visitStatement(node.tryBody);
       safeForInlining = saved;
       node.catchParameters.forEach(pushDominatingAssignment);
       node.catchBody = visitStatement(node.catchBody);
       node.catchParameters.forEach(popDominatingAssignment);
     });
     return node;
   }
 
   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());
       }
     }
     return node;
   }
 
   Expression visitThis(This node) {
     return node;
@@ skipping 13 matching lines @@
     return node;
   }
 
   Expression visitTypeOperator(TypeOperator node) {
     _rewriteList(node.typeArguments);
     node.value = visitExpression(node.value);
     return node;
   }
 
   Expression visitSetField(SetField node) {
+    allowRhsPropagation.add(true);
     node.value = visitExpression(node.value);
     node.object = visitExpression(node.object);
+    allowRhsPropagation.removeLast();
     return node;
   }
 
   Expression visitGetField(GetField node) {
     node.object = visitExpression(node.object);
     return node;
   }
 
   Expression visitGetStatic(GetStatic node) {
     return node;
   }
 
   Expression visitSetStatic(SetStatic node) {
+    allowRhsPropagation.add(true);
     node.value = visitExpression(node.value);
+    allowRhsPropagation.removeLast();
     return node;
   }
 
   Expression visitGetTypeTestProperty(GetTypeTestProperty node) {
     node.object = visitExpression(node.object);
     return node;
   }
 
   Expression visitCreateBox(CreateBox node) {
     return node;
@@ skipping 89 matching lines @@
   ///    var y = bar();
   ///    var z = y < x;
   ///
   ///      ==>
   ///
   ///    var z = foo() > bar();
   ///
   /// 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;
   }
 
   /// If [s] and [t] are similar statements we extract their subexpressions
   /// and returns a new statement of the same type using expressions combined
   /// with the [combine] callback. For example:
   ///
   ///   combineStatements(Return E1, Return E2) = Return combine(E1, E2)
   ///
   /// If [combine] returns E1 then the unified statement is equivalent to [s],
@@ skipping 390 matching lines @@
   VariableUseCallback callback;
 
   VariableUseVisitor(this.callback);
 
   visitVariableUse(VariableUse use) => callback(use);
 
   static void visit(Expression node, VariableUseCallback callback) {
     new VariableUseVisitor(callback).visitExpression(node);
   }
 }