Don't let HCheck instructions prevent GVN.

sdk/lib/_internal/compiler/implementation/ssa/nodes.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;
 
 abstract class HVisitor<R> {
   R visitAdd(HAdd node);
   R visitBailoutTarget(HBailoutTarget node);
   R visitBitAnd(HBitAnd node);
@@ skipping 602 matching lines @@
     from.usedBy.clear();
   }
 
   /**
    * Rewrites all uses of the [from] instruction to using either the
    * [to] instruction, or a [HCheck] instruction that has better type
    * information on [to], and that dominates the user.
    */
   void rewriteWithBetterUser(HInstruction from, HInstruction to) {
     Link<HCheck> better = const Link<HCheck>();
-    for (HInstruction user in to.usedBy) {
-      if (user is HCheck && identical((user as HCheck).checkedInput, to)) {
+    for (var user in to.usedBy) {
+      if (user != from && user is HCheck && user.checkedInput == to) {

[karlklose, 2013/06/12 13:12:38]

I would prefer:
for (HInstruction user in to.usedBy) {	
  if (user is! HCheck) continue;
  HCheck check = user;
  if (user.checkedInput == to) {
    better = better.prepend;
  }
}

[ngeoffray, 2013/06/12 13:55:09]

Done.

         better = better.prepend(user);
       }
     }
 
     if (better.isEmpty) return rewrite(from, to);
 
     L1: for (HInstruction user in from.usedBy) {
       for (HCheck check in better) {
         if (check.dominates(user)) {
           user.rewriteInput(from, check);
@@ skipping 177 matching lines @@
    * A pure instruction is an instruction that does not have any side
    * effect, nor any dependency. They can be moved anywhere in the
    * graph.
    */
   bool isPure() {
     return !sideEffects.hasSideEffects()
         && !sideEffects.dependsOnSomething()
         && !canThrow();
   }
 
+  // Overridden by [HCheck] to return the actual non-[HCheck]
+  // instruction it checks against.
+  HInstruction nonCheck() => this;
+
   // Can this node throw an exception?
   bool canThrow() => false;
 
   // Does this node potentially affect control flow.
   bool isControlFlow() => false;
 
   // All isFunctions work on the propagated types.
   bool isArray(Compiler compiler) =>
       instructionType.isArray(compiler);
   bool isReadableArray(Compiler compiler) =>
@@ skipping 62 matching lines @@
     // Check that the type and the sideEffects match.
     bool hasSameType = typeEquals(other);
     assert(hasSameType == (typeCode() == other.typeCode()));
     if (!hasSameType) return false;
     if (sideEffects != other.sideEffects) return false;
     // Check that the inputs match.
     final int inputsLength = inputs.length;
     final List<HInstruction> otherInputs = other.inputs;
     if (inputsLength != otherInputs.length) return false;
     for (int i = 0; i < inputsLength; i++) {
-      if (!identical(inputs[i], otherInputs[i])) return false;
+      if (!identical(inputs[i].nonCheck(), otherInputs[i].nonCheck())) {
+        return false;
+      }
     }
     // Check that the data in the instruction matches.
     return dataEquals(other);
   }
 
   int gvnHashCode() {
     int result = typeCode();
     int length = inputs.length;
     for (int i = 0; i < length; i++) {
-      result = (result * 19) + (inputs[i].id) + (result >> 7);
+      result = (result * 19) + (inputs[i].nonCheck().id) + (result >> 7);
     }
     return result;
   }
 
   // These methods should be overwritten by instructions that
   // participate in global value numbering.
   int typeCode() => HInstruction.UNDEFINED_TYPECODE;
   bool typeEquals(HInstruction other) => false;
   bool dataEquals(HInstruction other) => false;
 
@@ skipping 226 matching lines @@
  * instruction itself.
  */
 abstract class HCheck extends HInstruction {
   HCheck(inputs) : super(inputs) {
     setUseGvn();
   }
   HInstruction get checkedInput => inputs[0];
   bool isJsStatement() => true;
   bool canThrow() => true;
 
-  HInstruction unwrap() {
-    var checked = checkedInput;
-    while (checked is HCheck) checked = checked.checkedInput;
-    return checked;
-  }
+  HInstruction nonCheck() => checkedInput.nonCheck();
 }
 
 class HBailoutTarget extends HInstruction {
   final int state;
   bool isEnabled = true;
   // For each argument we record how many dummy (unused) arguments should
   // precede it, to make sure it lands in the correctly named parameter in the
   // bailout function.
   List<int> padding;
   HBailoutTarget(this.state) : super(<HInstruction>[]) {
@@ skipping 137 matching lines @@
   /**
    * Returns whether this call is on an interceptor object.
    */
   bool isCallOnInterceptor(Compiler compiler) {
     return isInterceptedCall && receiver.isInterceptor(compiler);
   }
 
   int typeCode() => HInstruction.INVOKE_DYNAMIC_TYPECODE;
   bool typeEquals(other) => other is HInvokeDynamic;
   bool dataEquals(HInvokeDynamic other) {
-    return selector == other.selector && element == other.element;
+    return selector.name == other.selector.name;

[Johnni Winther, 2013/06/12 11:06:08]

Why not [: selector == other.selector :] ?

[ngeoffray, 2013/06/12 13:55:09]

Because if we end up here, we know the receiver is the same, but the selectors
may not have been updated with the receiver type. So Selector.operator== will
return false, even though it's the same receiver.

   }
 }
 
 class HInvokeClosure extends HInvokeDynamic {
   HInvokeClosure(Selector selector, List<HInstruction> inputs)
     : super(selector, null, inputs) {
     assert(selector.isClosureCall());
   }
   accept(HVisitor visitor) => visitor.visitInvokeClosure(this);
 }
@@ skipping 1341 matching lines @@
   HBasicBlock get start => expression.start;
   HBasicBlock get end {
     // We don't create a switch block if there are no cases.
     assert(!statements.isEmpty);
     return statements.last.end;
   }
 
   bool accept(HStatementInformationVisitor visitor) =>
       visitor.visitSwitchInfo(this);
 }