Throw NoSuchMethod or ArgumentError instead of generating a bailout, when we know the next instruct…

sdk/lib/_internal/compiler/implementation/ssa/bailout.dart

 // Copyright (c) 2012, 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.
 
 part of ssa;
 
 class BailoutInfo {
   int instructionId;
   int bailoutId;
   BailoutInfo(this.instructionId, this.bailoutId);
@@ skipping 49 matching lines @@
  * Visits the graph in dominator order and inserts TypeGuards in places where
  * we consider the guard to be of value. This phase also does type
  * propagation to help find valuable type guards.
  */
 class SsaTypeGuardInserter extends SsaNonSpeculativeTypePropagator
     implements OptimizationPhase {
   final String name = 'SsaTypeGuardInserter';
   final CodegenWorkItem work;
   bool calledInLoop = false;
   bool isRecursiveMethod = false;
+  bool hasInsertedChecks = false;
   int stateId = 1;
   Map<HInstruction, HType> savedTypes = new Map<HInstruction, HType>();
 
   SsaTypeGuardInserter(compiler, this.work) : super(compiler);
 
   void visitGraph(HGraph graph) {
     // Run the speculative type propagator. This does in-place
     // update of the type of the instructions, and saves the
     // previous types in the [savedTypes] map.
     SsaTypePropagator propagator =
@@ skipping 103 matching lines @@
           && typeGuardWouldBeValuable(user, speculativeType)) {
         return true;
       }
     }
 
     // Insert type guards if the method is likely to be called in a
     // loop.
     return calledInLoop;
   }
 
+  // Returns whether an invcation of [selector] on [receiver] will throw a

[kasperl, 2013/04/23 06:12:43]

invcation -> invocation

[ngeoffray, 2013/04/23 08:06:04]

Done.

+  // [ArgumentError] if the argument is not of the right type.
+  bool willThrowArgumentError(Selector selector, HInstruction receiver) {
+    if (receiver != null && (receiver.isInteger() || receiver.isString())) {
+      return selector.isOperator() && selector.name != const SourceString('==');
+    }
+    return false;
+  }
+
+  // Returns whether an invocation of [selector] will throw a
+  // [NoSuchMethodError] if the receiver is not of the type
+  // [speculativeType].
+  bool willThrowNoSuchMethodErrorIfNot(Selector selector,
+                                       HType speculativeType) {
+    return compiler.world.hasSingleMatch(selector)
+        // In some cases, we want the receiver to be an integer,
+        // but that does not mean we will get a NoSuchMethodError
+        // if it's not: the receiver could be a double.
+        && !speculativeType.isInteger()
+        // We speculate on the [operator==] instruction, but we know it
+        // will never throw a [NoSuchMethodError].
+        && selector.name != const SourceString('==');
+  }
+
   bool shouldInsertTypeGuard(HInstruction instruction, HType speculativeType) {
     if (!speculativeType.isUseful()) return false;
     // If the types agree we don't need to check.
     if (speculativeType == instruction.instructionType) return false;
     // If a bailout check is more expensive than doing the actual operation
     // don't do it either.
     return typeGuardWouldBeValuable(instruction, speculativeType);
   }
 
   bool updateType(HInstruction instruction) {
     bool hasChanged = super.updateType(instruction);
     HType speculativeType = savedTypes[instruction];
     if (speculativeType == null) return hasChanged;
 
     if (shouldInsertTypeGuard(instruction, speculativeType)) {
       HInstruction insertionPoint;
       if (instruction is HPhi) {
         insertionPoint = instruction.block.first;
       } else if (instruction is HParameterValue) {
         // We insert the type guard at the end of the entry block
         // because if a parameter is live, it must be kept in the live
         // environment. Not doing so would mean we could visit a
         // parameter and remove it from the environment before
         // visiting a type guard.
         insertionPoint = instruction.block.last;
       } else {
         insertionPoint = instruction.next;
       }
-      // If the previous instruction is also a type guard, then both
-      // guards have the same environment, and can therefore share the
-      // same state id.
-      HBailoutTarget target;
-      int state;
-      if (insertionPoint.previous is HTypeGuard) {
-        HTypeGuard other = insertionPoint.previous;
-        target = other.bailoutTarget;
+
+      // Find out if we should actually just emit a check for it. If

[kasperl, 2013/04/23 06:12:43]

It would be nice if this could be refactored into a bunch of nicer helper
methods with good names.

[ngeoffray, 2013/04/23 08:06:04]

Done.

+      // there is a user of [instruction] in the same block (so that
+      // we know it will be executed), and that user has a selector
+      // (meaning it will be a call), we can put a type check instead
+      // of a type guard, that will either throw a [NoSuchMethodError]
+      // or a [ArgumentError].
+      bool willThrow = false;
+      Selector receiverSelectorOnThrow = null;
+      HInstruction firstUserWithSelector;
+      HInstruction firstUser;
+      // For a parameter, we look at the first block that contains
+      // user instructions.
+      HBasicBlock userMustBeInBlock = instruction is HParameterValue
+          ? instruction.block.successors[0]
+          : instruction.block;
+
+      // We distinguish between [firstUser] and
+      // [firstUserWithSelector] because we can often have the pattern
+      // that [firstUser] is a call to [: getInterceptor :] and
+      // [firstUserWithSelector] is the actual user of the
+      // instruction.
+      for (HInstruction user in instruction.usedBy) {

[kasperl, 2013/04/23 06:12:43]

Somehow it feels like we could use a better strategy here. If we inserted
throwing type guards in front of all throwing users we wouldn't need the
same-block restriction. We could always work on moving the throwing guards up
the dominator tree and eliminating redundant ones later (maybe that could be
done in general for type guards so we wouldn't have to insert them so early?).

Wouldn't that also eliminate the rather weird distinction between firstUser and
firstUserWithSelector?

[ngeoffray, 2013/04/23 08:06:04]

Agree. After discussing a bit about it, I'll work on another CL that will put
type guards at relevant use site, and moving them up when needed.
Actually, I don't need this distinction, I just changed the code to update all
users after the first user that has a selector.

+        if (user.block == userMustBeInBlock) {
+          if (firstUser == null || user.dominates(firstUser)) {
+            firstUser = user;
+            if (user.selector != null) {
+              firstUserWithSelector = user;
+            }
+          } else if (user.selector != null) {
+            if (firstUserWithSelector == null
+                || user.dominates(firstUserWithSelector)) {
+              firstUserWithSelector = user;
+            }
+          }
+        }
+      }
+
+      // If we have found a user with a selector, we find out if it
+      // will throw [NoSuchMethodError] or [ArgumentError].
+      if (firstUserWithSelector != null
+          && (firstUserWithSelector == firstUser
+              || firstUser.next == firstUserWithSelector)) {
+        assert(firstUser == firstUserWithSelector || !firstUser.hasSideEffects());

[kasperl, 2013/04/23 06:12:43]

Long line.

[ngeoffray, 2013/04/23 08:06:04]

Done.

+        Selector selector = firstUserWithSelector.selector;
+        HInstruction receiver = firstUserWithSelector.getDartReceiver(compiler);
+        if (receiver == instruction) {
+          if (willThrowNoSuchMethodErrorIfNot(selector, speculativeType)) {
+            receiverSelectorOnThrow = firstUserWithSelector.selector;
+            willThrow = true;
+          }
+        } else if (willThrowArgumentError(selector, receiver)) {
+          willThrow = true;
+        }
+      }
+
+      var check;
+      if (willThrow) {
+        insertionPoint = firstUser;
+        check = new HTypeConversion(
+            null,
+            receiverSelectorOnThrow == null
+                ? HTypeConversion.ARGUMENT_TYPE_CHECK
+                : HTypeConversion.RECEIVER_TYPE_CHECK,
+            speculativeType,
+            instruction,
+            receiverSelectorOnThrow);
+        hasInsertedChecks = true;
       } else {
-        state = stateId++;
-        target = new HBailoutTarget(state);
-        insertionPoint.block.addBefore(insertionPoint, target);
+        // If the previous instruction is also a type guard, then both
+        // guards have the same environment, and can therefore share the
+        // same state id.
+        HBailoutTarget target;
+        int state;
+        if (insertionPoint.previous is HTypeGuard) {
+          HTypeGuard other = insertionPoint.previous;
+          target = other.bailoutTarget;
+        } else {
+          state = stateId++;
+          target = new HBailoutTarget(state);
+          insertionPoint.block.addBefore(insertionPoint, target);
+        }
+        check = new HTypeGuard(speculativeType, instruction, target);
+        work.guards.add(check);
+        // By setting the type of the guard to the speculated type, we
+        // help the analysis find valuable type guards. This however
+        // requires to run a non-speculative type propagation again
+        // after this analysis.
+        check.instructionType = speculativeType;
       }
-      HTypeGuard guard = new HTypeGuard(speculativeType, instruction, target);
-      work.guards.add(guard);
-      // By setting the type of the guard to the speculated type, we
-      // help the analysis find valuable type guards. This however
-      // requires to run a non-speculative type propagation again
-      // after this analysis.
-      guard.instructionType = speculativeType;
-      instruction.block.rewrite(instruction, guard);
-      insertionPoint.block.addBefore(insertionPoint, guard);
+      instruction.block.rewrite(instruction, check);
+      insertionPoint.block.addBefore(insertionPoint, check);
     }
     return hasChanged;
   }
 }
 
 /**
  * Computes the environment for each SSA instruction: visits the graph
  * in post-dominator order. Removes an instruction from the environment
  * and adds its inputs to the environment at the instruction's
  * definition.
@@ skipping 367 matching lines @@
         hasComplexBailoutTargets = true;
       }
     } else {
       hasComplexBailoutTargets = true;
       blocks.forEach((HBasicBlock block) {
         block.bailoutTargets.add(target);
       });
     }
   }
 }