Optimize is-check to instanceof when it eliminates an interceptor

See http://dartbug.com/22016 for discussion

R=floitsch@google.com

Review URL: https://codereview.chromium.org//829913006

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@43058 260f80e4-7a28-3924-810f-c04153c831b5

pkg/compiler/lib/src/js_backend/backend.dart

@@ -1470,6 +1470,16 @@ class JavaScriptBackend extends Backend {
         element == jsFixedArrayClass;
   }
 
+  bool mayGenerateInstanceofCheck(DartType type) {
+    // We can use an instanceof check for raw types that have no subclass that
+    // is mixed-in or in an implements clause.
+
+    if (!type.isRaw) return false;
+    ClassElement classElement = type.element;
+    if (isInterceptorClass(classElement)) return false;
+    return compiler.world.hasOnlySubclasses(classElement);
+  }
+
   Element getExceptionUnwrapper() {
     return findHelper('unwrapException');
   }

pkg/compiler/lib/src/ssa/builder.dart

@@ -3442,7 +3442,8 @@ class SsaBuilder extends ResolvedVisitor {
       if (backend.hasDirectCheckFor(type)) {
         return new HIs.direct(type, expression, backend.boolType);
       }
-      // TODO(johnniwinther): Avoid interceptor if unneeded.
+      // The interceptor is not always needed.  It is removed by optimization
+      // when the receiver type or tested type permit.
       return new HIs.raw(
           type, expression, invokeInterceptor(expression), backend.boolType);
     }

pkg/compiler/lib/src/ssa/codegen.dart

@@ -2304,6 +2304,19 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
     if (!negative) push(new js.Prefix('!', pop()));
   }
 
+  void checkTypeViaInstanceof(
+      HInstruction input, DartType type, bool negative) {
+    registry.registerIsCheck(type);
+
+    use(input);
+
+    js.Expression jsClassReference =
+        backend.emitter.constructorAccess(type.element);
+    push(js.js('# instanceof #', [pop(), jsClassReference]));
+    if (negative) push(new js.Prefix('!', pop()));
+    registry.registerInstantiatedType(type);
+  }
+
   void handleNumberOrStringSupertypeCheck(HInstruction input,
                                           HInstruction interceptor,
                                           DartType type,
@@ -2421,6 +2434,10 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
         js.Expression numTest = pop();
         checkBigInt(input, relation);
         push(new js.Binary(negative ? '||' : '&&', numTest, pop()), node);
+      } else if (node.useInstanceOf) {
+        assert(interceptor == null);
+        checkTypeViaInstanceof(input, type, negative);
+        attachLocationToLast(node);
       } else if (Elements.isNumberOrStringSupertype(element, compiler)) {
         handleNumberOrStringSupertypeCheck(
             input, interceptor, type, negative: negative);

pkg/compiler/lib/src/ssa/codegen_helpers.dart

@@ -384,9 +384,14 @@ class SsaInstructionMerger extends HBaseVisitor {
     analyzeInputs(instruction, 0);
   }
 
-  // The codegen might use the input multiple times, so it must not be
-  // set generate at use site.
-  void visitIs(HIs instruction) {}
+  void visitIs(HIs instruction) {
+    // In the general case the input might be used multple multiple times, so it
+    // must not be set generate at use site.  If the code will generate
+    // 'instanceof' then we can generate at use site.
+    if (instruction.useInstanceOf) {
+      analyzeInputs(instruction, 0);
+    }
+  }
 
   // A bounds check method must not have its first input generated at use site,
   // because it's using it twice.

pkg/compiler/lib/src/ssa/interceptor_simplifier.dart

@@ -10,15 +10,18 @@ part of ssa;
  * 1) If the interceptor is for an object whose type is known, it
  * tries to use a constant interceptor instead.
  *
- * 2) It specializes interceptors based on the selectors it is being
- * called with.
+ * 2) Interceptors are specialized based on the selector it is used with.
  *
- * 3) If we know the object is not intercepted, we just use it
+ * 3) If we know the object is not intercepted, we just use the object
  * instead.
  *
- * 4) It replaces all interceptors that are used only once with
- * one-shot interceptors. It saves code size and makes the receiver of
- * an intercepted call a candidate for being generated at use site.
+ * 4) Single use interceptors at dynamic invoke sites are replaced with 'one
+ * shot interceptors' which are synthesized static helper functions that fetch
+ * the interceptor and then call the method.  This saves code size and makes the
+ * receiver of an intercepted call a candidate for being generated at use site.
+ *
+ * 5) Some HIs operations on an interceptor are replaced with a HIs version that
+ * uses 'instanceof' rather than testing a type flag.
  *
  */
 class SsaSimplifyInterceptors extends HBaseVisitor
@@ -56,6 +59,17 @@ class SsaSimplifyInterceptors extends HBaseVisitor
     if (!invoke.isInterceptedCall) return false;
     var interceptor = invoke.inputs[0];
     if (interceptor is! HInterceptor) return false;
+
+    // TODO(sra): Move this per-call code to visitInterceptor.
+    //
+    // The interceptor is visited first, so we get here only when the
+    // interceptor was not rewritten to a single shared replacement.  I'm not
+    // sure we should substitute a constant interceptor on a per-call basis if
+    // the interceptor is already available in a local variable, but it is
+    // possible that all uses can be rewritten to use different constants.
+
+    // TODO(sra): Also do self-interceptor rewrites on a per-use basis.
+
     HInstruction constant = tryComputeConstantInterceptor(
         invoke.inputs[1], interceptor.interceptedClasses);
     if (constant != null) {
@@ -238,8 +252,18 @@ class SsaSimplifyInterceptors extends HBaseVisitor
       }
     }
 
+    node.interceptedClasses = interceptedClasses;
+
     HInstruction receiver = node.receiver;
 
+    // TODO(sra): We should consider each use individually and then all uses
+    // together.  Each use might permit a different rewrite due to a refined
+    // receiver type.  Self-interceptor rewrites are always beneficial since the
+    // receiver is live at a invocation.  Constant-interceptor rewrites are only
+    // guaranteed to be beneficial if they can eliminate the need for the
+    // interceptor or reduce the uses to one that can be simplified with a
+    // one-shot interceptor or optimized is-check.
+
     if (canUseSelfForInterceptor(receiver, interceptedClasses)) {
       return rewriteToUseSelfAsInterceptor(node, receiver);
     }
@@ -252,32 +276,50 @@ class SsaSimplifyInterceptors extends HBaseVisitor
       return false;
     }
 
-    node.interceptedClasses = interceptedClasses;
-
-    // Try creating a one-shot interceptor.
+    // Try creating a one-shot interceptor or optimized is-check
     if (compiler.hasIncrementalSupport) return false;
     if (node.usedBy.length != 1) return false;
-    if (node.usedBy[0] is !HInvokeDynamic) return false;
+    HInstruction user = node.usedBy.single;
 
-    HInvokeDynamic user = node.usedBy[0];
-
-    // If [node] was loop hoisted, we keep the interceptor.
+    // If the interceptor [node] was loop hoisted, we keep the interceptor.
     if (!user.hasSameLoopHeaderAs(node)) return false;
 
-    // Replace the user with a [HOneShotInterceptor].
-    HConstant nullConstant = graph.addConstantNull(compiler);
-    List<HInstruction> inputs = new List<HInstruction>.from(user.inputs);
-    inputs[0] = nullConstant;
-    HOneShotInterceptor interceptor = new HOneShotInterceptor(
-        user.selector, inputs, user.instructionType, node.interceptedClasses);
-    interceptor.sourcePosition = user.sourcePosition;
-    interceptor.sourceElement = user.sourceElement;
+    bool replaceUserWith(HInstruction replacement) {
+      HBasicBlock block = user.block;
+      block.addAfter(user, replacement);
+      block.rewrite(user, replacement);
+      block.remove(user);
+      return false;
+    }
 
-    HBasicBlock block = user.block;
-    block.addAfter(user, interceptor);
-    block.rewrite(user, interceptor);
-    block.remove(user);
-    return true;
+    if (user is HIs) {
+      // See if we can rewrite the is-check to use 'instanceof', i.e. rewrite
+      // "getInterceptor(x).$isT" to "x instanceof T".
+      if (node == user.interceptor) {
+        JavaScriptBackend backend = compiler.backend;
+        if (backend.mayGenerateInstanceofCheck(user.typeExpression)) {
+          HInstruction instanceofCheck = new HIs.instanceOf(
+              user.typeExpression, user.expression, user.instructionType);
+          instanceofCheck.sourcePosition = user.sourcePosition;
+          instanceofCheck.sourceElement = user.sourceElement;
+          return replaceUserWith(instanceofCheck);
+        }
+      }
+    } else if (user is HInvokeDynamic) {
+      if (node == user.inputs[0]) {
+        // Replace the user with a [HOneShotInterceptor].
+        HConstant nullConstant = graph.addConstantNull(compiler);
+        List<HInstruction> inputs = new List<HInstruction>.from(user.inputs);
+        inputs[0] = nullConstant;
+        HOneShotInterceptor oneShotInterceptor = new HOneShotInterceptor(
+            user.selector, inputs, user.instructionType, interceptedClasses);
+        oneShotInterceptor.sourcePosition = user.sourcePosition;
+        oneShotInterceptor.sourceElement = user.sourceElement;
+        return replaceUserWith(oneShotInterceptor);
+      }
+    }
+
+    return false;
   }
 
   bool rewriteToUseSelfAsInterceptor(HInterceptor node, HInstruction receiver) {

pkg/compiler/lib/src/ssa/nodes.dart

@@ -2421,12 +2421,20 @@ class HIs extends HInstruction {
 
   final DartType typeExpression;
   final int kind;
+  final bool useInstanceOf;
 
   HIs.direct(DartType typeExpression,
              HInstruction expression,
              TypeMask type)
       : this.internal(typeExpression, [expression], RAW_CHECK, type);
 
+  // Pre-verified that the check can be done using 'instanceof'.
+  HIs.instanceOf(DartType typeExpression,
+                 HInstruction expression,
+                 TypeMask type)
+      : this.internal(typeExpression, [expression], RAW_CHECK, type,
+          useInstanceOf: true);
+
   HIs.raw(DartType typeExpression,
           HInstruction expression,
           HInterceptor interceptor,
@@ -2446,7 +2454,8 @@ class HIs extends HInstruction {
                TypeMask type)
       : this.internal(typeExpression, [expression, call], VARIABLE_CHECK, type);
 
-  HIs.internal(this.typeExpression, List<HInstruction> inputs, this.kind, type)
+  HIs.internal(this.typeExpression, List<HInstruction> inputs, this.kind,
+      TypeMask type, {bool this.useInstanceOf: false})
       : super(inputs, type) {
     assert(kind >= RAW_CHECK && kind <= VARIABLE_CHECK);
     setUseGvn();

tests/compiler/dart2js_extra/is_check_instanceof_test.dart

@@ -0,0 +1,69 @@
+// Copyright (c) 2015, 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.
+
+import "package:expect/expect.dart";
+
+// It is sometimes possible to compile is-checks to 'instanceof', when the class
+// is not in an 'implements' clause or used as a mixin.
+
+// This test verifies is-checks work with simple classes that have various
+// degrees of instantiation.
+
+class INSTANTIATED {}             // instantiated and used in many ways
+class DEFERRED {}                 // instantiated after first check
+class UNUSED {}                   // used only in is-check
+class REMOVED {}                  // allocated but optimized out of program
+class DEFERRED_AND_REMOVED {}     // allocated after first check and removed
+class USED_AS_TYPE_PARAMETER {}   // only used as a type parameter
+class USED_AS_TESTED_TYPE_PARAMETER {}   // only used as a type parameter
+
+class Check<T> {
+  bool check(x) => x is T;
+}
+
+class Check2<T> {
+  bool check(x) => x is USED_AS_TYPE_PARAMETER;
+}
+
+void main() {
+  var things = new List(3);
+  things.setRange(0, 3, [new INSTANTIATED(), 1, new Object()]);
+
+  var checkX = new Check<INSTANTIATED>();
+  var checkU1 = new Check<USED_AS_TESTED_TYPE_PARAMETER>();
+  var checkU2 = new Check<USED_AS_TYPE_PARAMETER>();
+
+  var removed = new REMOVED();  // This is optimized out.
+
+  // Tests that can be compiled to instanceof:
+  Expect.isTrue(things[0] is INSTANTIATED);
+  Expect.isFalse(things[1] is INSTANTIATED);
+  Expect.isFalse(things[1] is REMOVED);
+  Expect.isFalse(things[1] is DEFERRED_AND_REMOVED);
+  Expect.isFalse(things[1] is DEFERRED);
+  // Tests that might be optimized to false since there are no allocations:
+  Expect.isFalse(things[1] is UNUSED);
+  Expect.isFalse(things[1] is USED_AS_TYPE_PARAMETER);
+
+  Expect.isTrue(checkX.check(things[0]));
+  Expect.isFalse(checkX.check(things[1]));
+  Expect.isFalse(checkU1.check(things[1]));
+  Expect.isFalse(checkU2.check(things[1]));
+
+  var removed2 = new DEFERRED_AND_REMOVED();  // This is optimized out.
+
+  // First allocation of DEFERRED is after the above tests.
+  things.setRange(0, 3, [new INSTANTIATED(), 1, new DEFERRED()]);
+
+  // Tests that can be compiled to instanceof:
+  Expect.isTrue(things[0] is INSTANTIATED);
+  Expect.isFalse(things[1] is INSTANTIATED);
+  Expect.isFalse(things[1] is REMOVED);
+  Expect.isFalse(things[1] is DEFERRED_AND_REMOVED);
+  Expect.isFalse(things[1] is DEFERRED);
+  Expect.isTrue(things[2] is DEFERRED);
+  // Tests that might be optimized to false since there are no allocations:
+  Expect.isFalse(things[1] is UNUSED);
+  Expect.isFalse(things[1] is USED_AS_TYPE_PARAMETER);
+}