dart2js cps_ir: Use interceptors for is-checks (version 2)

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 ecf555d8352063689c44345637fc81cb1c18edd7..eaca68a49ba9efc9da41eba55405f9f7727359cc 100644
--- a/pkg/compiler/lib/src/cps_ir/type_propagation.dart
+++ b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
@@ -1806,8 +1806,29 @@ class TransformingVisitor extends LeafVisitor {
 
   Primitive visitTypeTest(TypeTest node) {
     Primitive prim = node.value.definition;
+
+    Primitive unaryBuiltinOperator(BuiltinOperator operator) =>
+        new ApplyBuiltinOperator(
+            operator, <Primitive>[prim], node.sourceInformation);
+
+    void unlinkInterceptor() {
+      if (node.interceptor != null) {
+        node.interceptor.unlink();
+        node.interceptor = null;
+      }

[sra1, 2015/10/07 04:01:28]

If instead of removing the interceptor, we add one if needed, then we are
generating

   letPrim p1 = TypeTest x in
     ...

->

   letPrim p2 = Interceptor x in
     letPrim p1 = TypeTestViaFlag p2 in
       ...

I'm not sure how to so that given this visitor method is called from
visitLetPrim for p1.  It seems that I can't get reanalyze to start from the
right place.

Perhaps a more general way to handle primitives is for each visitor to finish
the job by tail-calling into code equivalent to the continuation of visit() in
visitLetPrim when there is a single replacement.

The other thing I want to do is rewrite some tests as control flow.
Sometimes we can replace

  if (getInterceptor(x).$isFoo) ...

with

  if (x != null && x.$isFoo) ...

[Kevin Millikin (Google), 2015/10/07 14:10:49]

Yeah, that corresponds to replacing a single instruction with a pair of
instructions.  Since this pass was originally sparse conditional constant
propagation, we didn't really build it so that was easy.

I wonder if we really want to do that in this pass anyway?  At least in this
case, it seems like only two of the transformations have anything to do with the
type computed for 'prim' --- the one that replace an 'is int' check with
Math.floor when prim is known to be a number, and the one that replaces an 'is
int' check with typeof === 'number' when prim is known to not be a double.

It seems like the rest of the transformations could either be done at
construction time (if they don't occur due to other optimizations) or else as a
separate pass run before this one or as part of shrinking reductions (if the
pattern can occur as a result of other optimizations).  Is there any
optimization that changes the dartType of an TypeTest?

[sra1, 2015/10/07 17:45:58]

The dartType never changes.  The rewrite depends on both the dartType and the
input type.

+    }
+
     AbstractValue value = getValue(prim);
-    if (node.dartType == dartTypes.coreTypes.intType) {
+    types.DartType dartType = node.dartType;
+
+    if (!(dartType.isInterfaceType && dartType.isRaw)) {
+      // TODO(23685): Efficient function arity check.
+      // TODO(sra): Pass interceptor to runtime subtype functions.
+      unlinkInterceptor();
+      return null;
+    }
+
+    if (dartType == dartTypes.coreTypes.intType) {
       // Compile as typeof x === 'number' && Math.floor(x) === x
       if (lattice.isDefinitelyNum(value, allowNull: true)) {
         // If value is null or a number, we can skip the typeof test.
@@ -1819,23 +1840,50 @@ class TransformingVisitor extends LeafVisitor {
       if (lattice.isDefinitelyNotNonIntegerDouble(value)) {
         // If the value cannot be a non-integer double, but might not be a
         // number at all, we can skip the Math.floor test.
-        return new ApplyBuiltinOperator(
-            BuiltinOperator.IsNumber,
-            <Primitive>[prim],
-            node.sourceInformation);
+        return unaryBuiltinOperator(BuiltinOperator.IsNumber);
       }
       return new ApplyBuiltinOperator(
           BuiltinOperator.IsNumberAndFloor,
           <Primitive>[prim, prim, prim],
           node.sourceInformation);
     }
-    if (node.dartType == dartTypes.coreTypes.numType ||
-        node.dartType == dartTypes.coreTypes.doubleType) {
-      return new ApplyBuiltinOperator(
-          BuiltinOperator.IsNumber,
-          <Primitive>[prim],
-          node.sourceInformation);
+    if (dartType == dartTypes.coreTypes.numType ||
+        dartType == dartTypes.coreTypes.doubleType) {
+      return unaryBuiltinOperator(BuiltinOperator.IsNumber);
+    }
+
+    if (dartType.element == functionCompiler.glue.jsFixedArrayClass) {
+      // TODO(sra): Check input is restricted to JSArray.
+      return unaryBuiltinOperator(BuiltinOperator.IsFixedLengthJSArray);
+    }
+
+    if (dartType.element == functionCompiler.glue.jsExtendableArrayClass) {
+      // TODO(sra): Check input is restricted to JSArray.
+      return unaryBuiltinOperator(BuiltinOperator.IsExtendableJSArray);
+    }
+
+    if (dartType.element == functionCompiler.glue.jsMutableArrayClass) {
+      // TODO(sra): Check input is restricted to JSArray.
+      return unaryBuiltinOperator(BuiltinOperator.IsModifiableJSArray);
+    }
+
+    if (dartType.element == functionCompiler.glue.jsUnmodifiableArrayClass) {
+      // TODO(sra): Check input is restricted to JSArray.
+      return unaryBuiltinOperator(BuiltinOperator.IsUnmodifiableJSArray);
+    }
+
+    if (dartType == dartTypes.coreTypes.stringType ||
+        dartType == dartTypes.coreTypes.boolType) {
+      // These types are recognized in tree_ir TypeOperator codegen.
+      unlinkInterceptor();
+      return null;
     }
+
+    // TODO(sra): Propagate sourceInformation.
+    return new TypeTestViaFlag(node.interceptor.definition, dartType);
+  }
+
+  Primitive visitTypeTestViaFlag(TypeTestViaFlag node) {
     return null;
   }
 
@@ -2316,9 +2364,20 @@ class TypePropagationVisitor implements Visitor {
   }
 
   void visitTypeTest(TypeTest node) {
-    AbstractValue input = getValue(node.value.definition);
+    handleTypeTest(node, getValue(node.value.definition), node.dartType);
+  }
+
+  void visitTypeTestViaFlag(TypeTestViaFlag node) {
+    // TODO(sra): We could see if we can find the value in the interceptor
+    // expression.
+    setValue(node, nonConstant(typeSystem.boolType));
+    //handleTypeTest(node, getValue(node.value.definition), node.dartType);
+  }
+
+  void handleTypeTest(
+      Primitive node, AbstractValue input, types.DartType dartType) {
     TypeMask boolType = typeSystem.boolType;
-    switch(lattice.isSubtypeOf(input, node.dartType, allowNull: false)) {
+    switch(lattice.isSubtypeOf(input, dartType, allowNull: false)) {
       case AbstractBool.Nothing:
         break; // And come back later.