Remove forced type changes when they can't deopt

src/hydrogen-representation-changes.cc

Index: src/hydrogen-representation-changes.cc
diff --git a/src/hydrogen-representation-changes.cc b/src/hydrogen-representation-changes.cc
index b916d9b302fa443d438510ebcfc99e32fd37f5d5..eb9c3ea6282f648a38cc2a47b6bf95cff80824b2 100644
--- a/src/hydrogen-representation-changes.cc
+++ b/src/hydrogen-representation-changes.cc
@@ -51,6 +51,18 @@ void HRepresentationChangesPhase::InsertRepresentationChangeForUse(
 }
 
 
+static bool HChangeCanDeoptForUse(HChange* change, HValue* use, int use_index) {

[Jakob Kummerow, 2014/06/12 10:16:49]

I don't like that this function's name doesn't match what it computes. E.g. a
smi-to-int32 change never deopts, but on arm/ia32 this function says it can.
What you're really interested in are unnecessary int32-to-smi conversions, and I
can't imagine any useful future generalizations. So I'd suggest to:
- rename this function to IsNonDeoptingIntToSmiChange
- modify the comment at the call site to describe this specialized purpose
- simplify the implementation a bit:
return from_rep.IsInt32() && to_rep.IsSmi() && SmiValuesAre32Bits();

+  Representation change_rep = change->RequiredInputRepresentation(0);

[Jakob Kummerow, 2014/06/12 10:16:49]

It's cleaner to use change->from()...

+  Representation use_rep = use->RequiredInputRepresentation(use_index);

[Jakob Kummerow, 2014/06/12 10:16:49]

...and change->to().

+  if (change_rep.IsSmiOrInteger32() && use_rep.IsSmiOrInteger32() &&
+      (change->CheckFlag(HValue::kUint32) == use->CheckFlag(HValue::kUint32)) &&

[Jakob Kummerow, 2014/06/12 10:16:49]

Representation changes are inserted before the uint32 analysis phase (which sets
kUint32 flags), so this should just be an ASSERT (with a comment about phase
ordering).

+      SmiValuesAre32Bits()) {
+    return false;
+  }
+  return true;
+}
+
+
 void HRepresentationChangesPhase::InsertRepresentationChangesForValue(
     HValue* value) {
   Representation r = value->representation();
@@ -65,17 +77,32 @@ void HRepresentationChangesPhase::InsertRepresentationChangesForValue(
     int use_index = it.index();
     Representation req = use_value->RequiredInputRepresentation(use_index);
     if (req.IsNone() || req.Equals(r)) continue;
+
+    // If this is an HForceRepresentation instruction, and an HChange has been
+    // inserted above it, examine the input representation of the HChange to see
+    // if it satisfies any of the HForceRepresentation's uses. If so, and the
+    // HChange can't deopt, set the input of the use to the input of the
+    // HChange.
+    if (value->IsForceRepresentation()) {
+      HValue* parent = HForceRepresentation::cast(value)->value();

[Jakob Kummerow, 2014/06/12 10:16:49]

nit: we don't use "parent" for instructions (they form a directed graph, not a
tree). Maybe "input"?

+      if (parent->IsChange() &&
+          parent->RequiredInputRepresentation(0).Equals(req) &&

[Jakob Kummerow, 2014/06/12 10:16:49]

again, s/parent->RequiredInputRepresentation(0)/HChange::cast(input)->from()/

+          !HChangeCanDeoptForUse(HChange::cast(parent), value, 0)) {
+        use_value->SetOperandAt(use_index, HChange::cast(parent)->value());
+        continue;
+      }
+    }
     InsertRepresentationChangeForUse(value, use_value, use_index, req);
   }
   if (value->HasNoUses()) {
-    ASSERT(value->IsConstant());
+    ASSERT(value->IsConstant() || value->IsForceRepresentation());
     value->DeleteAndReplaceWith(NULL);
-  }
-
-  // The only purpose of a HForceRepresentation is to represent the value
-  // after the (possible) HChange instruction.  We make it disappear.
-  if (value->IsForceRepresentation()) {
-    value->DeleteAndReplaceWith(HForceRepresentation::cast(value)->value());
+  } else {
+    // The only purpose of a HForceRepresentation is to represent the value
+    // after the (possible) HChange instruction.  We make it disappear.
+    if (value->IsForceRepresentation()) {
+      value->DeleteAndReplaceWith(HForceRepresentation::cast(value)->value());
+    }
   }
 }