dart2js cps: Fuse tear-off and call invocation.

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

Index: pkg/compiler/lib/src/cps_ir/type_propagation.dart
diff --git a/pkg/compiler/lib/src/cps_ir/type_propagation.dart b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
index 578589bafdbf0a21863a4c0fd782b5a2af830acf..c9223d435a91b8bc44c442c6c7382f1dfac2b652 100644
--- a/pkg/compiler/lib/src/cps_ir/type_propagation.dart
+++ b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
@@ -53,6 +53,10 @@ class TypeMaskSystem {
     return mask.locateSingleElement(selector, mask, classWorld.compiler);
   }
 
+  bool needsNoSuchMethodHandling(TypeMask mask, Selector selector) {
+    return mask.needsNoSuchMethodHandling(selector, classWorld);
+  }
+
   TypeMask getReceiverType(MethodElement method) {
     assert(method.isInstanceMember);
     return nonNullSubclass(method.enclosingClass);
@@ -477,6 +481,12 @@ class TransformingVisitor extends RecursiveVisitor {
     visit(root);
   }
 
+  /// Sets parent pointers and computes types for the given subtree.
+  void reanalyze(Node node) {
+    new ParentVisitor().visit(node);
+    analyzer.reanalyzeSubtree(node);
+  }
+
   /// Removes the entire subtree of [node] and inserts [replacement].
   /// All references in the [node] subtree are unlinked, and parent pointers
   /// in [replacement] are initialized and its types recomputed.
@@ -484,14 +494,16 @@ class TransformingVisitor extends RecursiveVisitor {
   /// [replacement] must be "fresh", i.e. it must not contain significant parts
   /// of the original IR inside of it since the [ParentVisitor] will
   /// redundantly reprocess it.
-  void replaceSubtree(Expression node, Expression replacement) {
+  void replaceSubtree(Expression node, Expression replacement,

[floitsch, 2015/07/02 18:02:41]

add comment about "unlink".

[asgerf, 2015/07/03 08:44:46]

Done.

+                      {bool unlink: true}) {
     InteriorNode parent = node.parent;
     parent.body = replacement;
     replacement.parent = parent;
     node.parent = null;
-    RemovalVisitor.remove(node);
-    new ParentVisitor().visit(replacement);
-    analyzer.reanalyzeSubtree(replacement);
+    if (unlink) {
+      RemovalVisitor.remove(node);
+    }
+    reanalyze(replacement);
   }
 
   /// Make a constant primitive for [constant] and set its entry in [values].
@@ -729,10 +741,106 @@ class TransformingVisitor extends RecursiveVisitor {
     }
   }
 
+  /// If [prim] is the parameter to a call continuation, returns the
+  /// corresponding call.
+  Invoke getInvocationWithResult(Primitive prim) {
+    if (prim is Parameter && prim.parent is Continuation) {
+      Continuation cont = prim.parent;
+      if (cont.hasExactlyOneUse) {
+        Node use = cont.firstRef.parent;
+        if (use is Invoke) {
+          return use;
+        }
+      }
+    }
+    return null;
+  }
+
+  /// Rewrites an invocation of a torn-off method into a method call directly
+  /// on the receiver. For example:
+  ///
+  ///     obj.get$foo().call$<n>(<args>)
+  ///       =>
+  ///     obj.foo$<n>(<args>)
+  ///
+  bool specializeClosureCall(InvokeMethod node) {
+    Selector call = node.selector;
+    if (!call.isClosureCall) return false;
+
+    assert(!isInterceptedSelector(call));
+    assert(call.argumentCount == node.arguments.length);
+
+    Primitive tearOff = node.receiver.definition;
+    // Note: We don't know if [tearOff] is actually a tear-off.
+    // We name variables based on the pattern we are trying to match.
+
+    if (tearOff is GetStatic && tearOff.element.isFunction) {
+      FunctionElement target = tearOff.element;
+      FunctionSignature signature = target.functionSignature;
+
+      // If the selector does not apply, don't bother (will throw at runtime).
+      if (!call.signatureApplies(target)) return false;
+
+      // If some optional arguments are missing, don't bother (although we
+      // could insert default arguments here to complete the optimization).
+      if (call.argumentCount != signature.parameterCount) return false;

[floitsch, 2015/07/02 18:02:40]

Maybe add a TODO?

[asgerf, 2015/07/03 08:44:46]

Done.

+
+      InvokeStatic invoke = new InvokeStatic.byReference(
+          target,
+          new Selector.fromElement(target),
+          node.arguments,
+          node.continuation,
+          node.sourceInformation);
+      node.receiver.unlink();
+      replaceSubtree(node, invoke, unlink: false);
+      visitInvokeStatic(invoke);
+      return true;
+    }
+    Invoke tearOffInvoke = getInvocationWithResult(tearOff);
+    if (tearOffInvoke is InvokeMethod && tearOffInvoke.selector.isGetter) {
+      Selector getter = tearOffInvoke.selector;
+      Continuation getterCont = tearOffInvoke.continuation.definition;
+      Primitive object = tearOffInvoke.receiver.definition;
+
+      // Ensure that the object actually has a foo member, since we might
+      // otherwise alter a noSuchMethod call.
+      TypeMask type = getValue(object).type;
+      if (typeSystem.needsNoSuchMethodHandling(type, getter)) return false;
+
+      // Determine if the getter invocation can have side-effects.
+      Element element = typeSystem.locateSingleElement(type, getter);
+      bool isPure = element != null && !element.isGetter;
+
+      // If there are multiple uses, we cannot eliminate the getter call and
+      // therefore risk duplicating its side effects.
+      if (!isPure && tearOff.hasMultipleUses) return false;
+
+      // If we are not the immediate successor of the getter call, we risk
+      // changing the time of evaluation of an impure call.
+      if (!isPure && getterCont.body != node) return false;
+
+      node.selector =

[floitsch, 2015/07/02 18:02:40]

I'm not a fan of modifying the fields of a node.
Would it make sense to create a new Invoke node and replace the old one with it?

[asgerf, 2015/07/03 08:44:46]

Fine with me.

+        new Selector(SelectorKind.CALL, getter.memberName, call.callStructure);
+      node.receiver.changeTo(object);
+      node.mask = type;
+
+      if (tearOff.hasNoUses) {

[floitsch, 2015/07/02 18:02:40]

Would this happen in a different optimization by itself?

[asgerf, 2015/07/03 08:44:46]

In some cases it could (but currently not implemented) but in general no,
because the getter call we eliminate has side effects in some cases.

There is a comment before one of the ifs above explaining why, but I have
elaborated on the comment down here to clear up any confusion.

+        // Eliminate the getter call if it has no more uses.
+        getterCont.parameters.clear();
+        replaceSubtree(tearOffInvoke, new InvokeContinuation(getterCont, []));
+      }
+
+      visitInvokeMethod(node);
+      return true;
+    }
+    return false;
+  }
+
   void visitInvokeMethod(InvokeMethod node) {
     if (constifyExpression(node)) return;
     if (specializeOperatorCall(node)) return;
     if (specializeFieldAccess(node)) return;
+    if (specializeClosureCall(node)) return;
 
     AbstractValue receiver = getValue(node.receiver.definition);
     node.receiverIsNotNull = receiver.isDefinitelyNotNull;
@@ -846,6 +954,7 @@ class TransformingVisitor extends RecursiveVisitor {
             node.arguments[k] = null; // Remove the argument after the loop.
           }
           node.arguments[startOfSequence] = new Reference<Primitive>(prim);
+          node.arguments[startOfSequence].parent = node;

[asgerf, 2015/07/02 16:50:36]

Drive-by fix, not really related.

           argumentsWereRemoved = true;
         }
         if (argumentsWereRemoved) {
@@ -909,16 +1018,15 @@ class TransformingVisitor extends RecursiveVisitor {
       newPrim.substituteFor(node.primitive);
       RemovalVisitor.remove(node.primitive);
       node.primitive = newPrim;
+      newPrim.parent = node;
     } else {
       Primitive newPrim = visit(node.primitive);
       if (newPrim != null) {
-        if (!values.containsKey(newPrim)) {
-          // If the type was not set, default to the same type as before.
-          values[newPrim] = values[node.primitive];
-        }
         newPrim.substituteFor(node.primitive);
         RemovalVisitor.remove(node.primitive);
         node.primitive = newPrim;
+        newPrim.parent = node;
+        reanalyze(newPrim);
       }
       if (node.primitive.hasNoUses && node.primitive.isSafeForElimination) {
         // Remove unused primitives before entering the body.