dart2js cps: More interceptor optimizations and fixes.

pkg/compiler/lib/src/cps_ir/share_interceptors.dart

Index: pkg/compiler/lib/src/cps_ir/share_interceptors.dart
diff --git a/pkg/compiler/lib/src/cps_ir/share_interceptors.dart b/pkg/compiler/lib/src/cps_ir/share_interceptors.dart
index 3706725e3d102a8c63b652ec7fe196761d9e435c..205e332b44e2ab030c84314ca709939c368c55d4 100644
--- a/pkg/compiler/lib/src/cps_ir/share_interceptors.dart
+++ b/pkg/compiler/lib/src/cps_ir/share_interceptors.dart
@@ -7,10 +7,15 @@ library dart2js.cps_ir.share_interceptors;
 import 'optimizers.dart';
 import 'cps_ir_nodes.dart';
 import 'loop_hierarchy.dart';
+import 'cps_fragment.dart';
 import '../constants/values.dart';
+import '../elements/elements.dart';
+import '../js_backend/js_backend.dart' show JavaScriptBackend;
+import '../types/types.dart' show TypeMask;
+import '../io/source_information.dart' show SourceInformation;
 
 /// Removes redundant `getInterceptor` calls.
-/// 
+///
 /// The pass performs three optimizations for interceptors:
 ///- pull interceptors out of loops
 ///- replace interceptors with constants
@@ -22,21 +27,21 @@ class ShareInterceptors extends TrampolineRecursiveVisitor implements Pass {
   final Map<Primitive, Continuation> loopHeaderFor =
       <Primitive, Continuation>{};
 
-  /// An interceptor currently in scope for a given primitive.
-  final Map<Primitive, Primitive> interceptorFor =
-      <Primitive, Primitive>{};
+  /// An interceptor currently in scope for a given primitive or constant value.
+  final Map<dynamic, Primitive> interceptorFor = <dynamic, Primitive>{};
 
-  /// A primitive currently in scope holding a given interceptor constant.
-  final Map<ConstantValue, Primitive> sharedConstantFor =
-      <ConstantValue, Primitive>{};
-
-  /// Interceptors to be hoisted out of the given loop.
-  final Map<Continuation, List<Primitive>> loopHoistedInterceptors =
-      <Continuation, List<Primitive>>{};
+  /// Primitives and constant values that have been hoisted out of the given
+  /// loop.  These are keys in [interceptorFor] that must be removed when we
+  /// exit the loop.
+  final Map<Continuation, List<dynamic>> loopHoistedInterceptors =
+      <Continuation, List<dynamic>>{};
 
+  JavaScriptBackend backend;
   LoopHierarchy loopHierarchy;
   Continuation currentLoopHeader;
 
+  ShareInterceptors(this.backend);
+
   void rewrite(FunctionDefinition node) {
     loopHierarchy = new LoopHierarchy(node);
     visit(node.body);
@@ -57,19 +62,11 @@ class ShareInterceptors extends TrampolineRecursiveVisitor implements Pass {
         // After the loop body has been processed, all interceptors hoisted
         // to this loop fall out of scope and should be removed from the
         // environment.
-        List<Primitive> hoisted = loopHoistedInterceptors[cont];
+        List hoisted = loopHoistedInterceptors[cont];
         if (hoisted != null) {
-          for (Primitive interceptor in hoisted) {
-            if (interceptor is Interceptor) {
-              Primitive input = interceptor.input.definition;
-              assert(interceptorFor[input] == interceptor);
-              interceptorFor.remove(input);
-            } else if (interceptor is Constant) {
-              assert(sharedConstantFor[interceptor.value] == interceptor);
-              sharedConstantFor.remove(interceptor.value);
-            } else {
-              throw "Unexpected interceptor: $interceptor";
-            }
+          for (var input in hoisted) {
+            assert(interceptorFor.containsKey(input));
+            interceptorFor.remove(input);
           }
         }
       });
@@ -77,6 +74,72 @@ class ShareInterceptors extends TrampolineRecursiveVisitor implements Pass {
     return cont.body;
   }
 
+  /// If only one method table can be returned by the given interceptor,
+  /// returns a constant for that method table.
+  InterceptorConstantValue getInterceptorConstant(Interceptor node) {
+    if (node.interceptedClasses.length != 1) {
+      return null;
+    }
+    ClassElement interceptorClass = node.interceptedClasses.single;
+    if (Elements.isNativeOrExtendsNative(interceptorClass)) {
+      // There might exist self-intercepting subclasses of native classes.

[sra1, 2015/10/21 00:46:25]

I don't think this is true.  Custom elements should be intercepted.

[sra1, 2015/10/21 01:29:38]

Specifically, this is an important regression:


      $index: function(receiver, index) {
        return C.NodeList_methods.$index(this._nodeList, index);
      },
-->
      $index: function(receiver, index) {
        var v0 = this._nodeList;
        return J.getInterceptor$x(v0).$index(v0, index);
      },

[asgerf, 2015/10/21 11:52:46]

Should be fixed now.

[sra1, 2015/10/21 21:46:56]

I'm still seeing some cases of regressions:

      eraseSplashScreen$0: function() {
        var splash = C.HtmlDocument_methods.querySelector$1(document,
"#appSplash");
        if (splash)
          J.getInterceptor$x(splash).remove$0(splash);
      },
-->
      eraseSplashScreen$0: function() {
        var v0 = document, splash = J.getInterceptor$x(v0).querySelector$1(v0,
"#appSplash");
        if (splash)
          J.getInterceptor$x(splash).remove$0(splash);
      },

+      return null;
+    }
+    return new InterceptorConstantValue(interceptorClass.rawType);
+  }
+
+  bool hasNoFalsyValues(ClassElement class_) {
+    return class_ != backend.jsInterceptorClass &&
+       class_ != backend.jsNullClass &&
+       class_ != backend.jsBoolClass &&
+       class_ != backend.jsStringClass &&
+       !class_.isSubclassOf(backend.jsNumberClass);
+  }
+
+  Continuation getCurrentOuterLoop({Continuation scope}) {
+    Continuation inner = null, outer = currentLoopHeader;
+    while (outer != scope) {
+      inner = outer;
+      outer = loopHierarchy.getEnclosingLoop(outer);
+    }
+    return inner;
+  }
+
+  /// Binds the given constant in a primitive, in scope of the [useSite].
+  ///
+  /// The constant will be hoisted out of loops, and shared with other requests
+  /// for the same constant as long as it is in scope.
+  Primitive makeConstantFor(ConstantValue constant,
+                            {Expression useSite,
+                             TypeMask type,
+                             SourceInformation sourceInformation,
+                             Entity hint}) {
+    Constant prim = interceptorFor[constant];
+    if (prim != null) {
+      return prim;
+    }
+    prim = new Constant(constant, sourceInformation: sourceInformation);
+    prim.hint = hint;
+    prim.type = type;
+    interceptorFor[constant] = prim;
+    LetPrim letPrim = new LetPrim(prim);
+    Continuation loop = getCurrentOuterLoop();
+    if (loop != null) {
+      LetCont loopBinding = loop.parent;
+      letPrim.insertAbove(loopBinding);
+      loopHoistedInterceptors
+          .putIfAbsent(loop, () => [])
+          .add(constant);
+    } else {
+      letPrim.insertAbove(useSite);
+      pushAction(() {
+        assert(interceptorFor[constant] == prim);
+        interceptorFor.remove(constant);
+      });
+    }
+    return prim;
+  }
+
   @override
   Expression traverseLetPrim(LetPrim node) {
     loopHeaderFor[node.primitive] = currentLoopHeader;
@@ -102,74 +165,82 @@ class ShareInterceptors extends TrampolineRecursiveVisitor implements Pass {
     // There is no interceptor obtained from this particular input, but
     // there might one obtained from another input that is known to
     // have the same result, so try to reuse that.
-    InterceptorConstantValue constant = interceptor.constantValue;
-    if (constant != null) {
-      existing = sharedConstantFor[constant];
-      if (existing != null) {
-        existing.substituteFor(interceptor);
-        interceptor.destroy();
-        node.remove();
-        return next;
-      }
-
-      // The interceptor could not be shared. Replace it with a constant.
-      Constant constantPrim = new Constant(constant);
-      node.primitive = constantPrim;
-      constantPrim.parent = node;
-      constantPrim.hint = interceptor.hint;
-      constantPrim.type = interceptor.type;
+    InterceptorConstantValue constant = getInterceptorConstant(interceptor);
+    if (constant != null && interceptor.isAlwaysIntercepted) {
+      Primitive constantPrim = makeConstantFor(constant,
+          useSite: node,
+          type: interceptor.type,
+          sourceInformation: interceptor.sourceInformation);
+      constantPrim.useElementAsHint(interceptor.hint);
       constantPrim.substituteFor(interceptor);
       interceptor.destroy();
-      sharedConstantFor[constant] = constantPrim;
-    } else {
-      interceptorFor[input] = interceptor;
+      node.remove();
+      return next;
     }
 
-    // Determine the outermost loop where the input to the interceptor call
-    // is available.  Constant interceptors take no input and can thus be
-    // hoisted all way to the top-level.
-    Continuation referencedLoop = constant != null
-        ? null
-        : lowestCommonAncestor(loopHeaderFor[input], currentLoopHeader);
+    Primitive interceptorValue = interceptor;
+
+    // Determine how far the interceptor can be lifted. The outermost loop
+    // that contains the input binding should also contain the interceptor
+    // binding.
+    Expression insertionPoint = node;
+    Continuation hoistTarget;
+    Continuation referencedLoop =
+        lowestCommonAncestor(loopHeaderFor[input], currentLoopHeader);
     if (referencedLoop != currentLoopHeader) {
-      // [referencedLoop] contains the binding for [input], so we cannot hoist
-      // the interceptor outside that loop.  Find the loop nested one level
-      // inside referencedLoop, and hoist the interceptor just outside that one.
-      Continuation loop = currentLoopHeader;
-      Continuation enclosing = loopHierarchy.getEnclosingLoop(loop);
-      while (enclosing != referencedLoop) {
-        assert(loop != null);
-        loop = enclosing;
-        enclosing = loopHierarchy.getEnclosingLoop(loop);
-      }
-      assert(loop != null);
+      hoistTarget = getCurrentOuterLoop(scope: referencedLoop);
+      LetCont letCont = hoistTarget.parent;
+      insertionPoint = letCont;
+    }
 
-      // Move the LetPrim above the loop binding.
-      LetCont loopBinding = loop.parent;
+    // If the interceptor is either a constant or null, replace the expression
+    // with a constant guarded by a null-check.
+    if (constant != null && interceptor.isAlwaysNullOrIntercepted) {
+      Primitive constantPrim = makeConstantFor(constant,
+          useSite: node,
+          type: interceptor.type.nonNullable(),
+          sourceInformation: interceptor.sourceInformation);
+      CpsFragment cps = new CpsFragment(interceptor.sourceInformation);
+      Parameter param = new Parameter(interceptor.hint);
+      Continuation cont = cps.letCont(<Parameter>[param]);
+      if (interceptor.interceptedClasses.every(hasNoFalsyValues)) {
+        // If null is the only falsy value, compile as "x && CONST".
+        cps.ifFalsy(input).invokeContinuation(cont, [input]);
+      } else {
+        // If there are other falsy values compile as "x == null ? x : CONST".
+        Primitive condition = cps.applyBuiltin(
+            BuiltinOperator.LooseEq,
+            [input, cps.makeNull()]);
+        cps.ifTruthy(condition).invokeContinuation(cont, [input]);
+      }
+      cps.invokeContinuation(cont, [constantPrim]);
+      cps.context = cont;
+      cps.insertAbove(insertionPoint);
+      param.substituteFor(interceptor);
+      interceptor.destroy();
       node.remove();
-      node.insertAbove(loopBinding);
-
-      // A different loop now contains the interceptor.
-      loopHeaderFor[node.primitive] = enclosing;
+      interceptorValue = param;
+    } else if (hoistTarget != null) {
+      // No constify optimization applied, but we can hoist the interceptor
+      // call out of the loop.
+      node.remove();
+      node.insertAbove(insertionPoint);
+    }
 
-      // Register the interceptor as hoisted to that loop, so it will be
-      // removed from the environment when it falls out of scope.
+    // Put the interceptor in the environment, and make sure it gets removed
+    // when the binding falls out of scope.
+    interceptorFor[input] = interceptorValue;
+    if (hoistTarget != null) {
       loopHoistedInterceptors
-          .putIfAbsent(loop, () => <Primitive>[])
-          .add(node.primitive);
-    } else if (constant != null) {
-      // The LetPrim was not hoisted. Remove the bound interceptor from the
-      // environment when leaving the LetPrim body.
-      pushAction(() {
-        assert(sharedConstantFor[constant] == node.primitive);
-        sharedConstantFor.remove(constant);
-      });
+          .putIfAbsent(hoistTarget, () => [])
+          .add(input);
     } else {
       pushAction(() {
-        assert(interceptorFor[input] == node.primitive);
+        assert(interceptorFor[input] == interceptorValue);
         interceptorFor.remove(input);
       });
     }
+
     return next;
   }