Remove forced type changes when they can't deopt
authorm.m.capewell@googlemail.com <m.m.capewell@googlemail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 12 Jun 2014 16:47:51 +0000 (16:47 +0000)
committerm.m.capewell@googlemail.com <m.m.capewell@googlemail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 12 Jun 2014 16:47:51 +0000 (16:47 +0000)
Hydrogen attempts to force representation changes on certain operations in order
to deoptimise on the change rather than the operation. However, these forced
changes are often unnecessary on 64-bit platforms, and cause poor code
generation, so this patch makes some of them conditional on whether it's
possible for deoptimisation to occur in the change.

On ARM64, this prevents sequences like:
                  ;;; <@46,#89> smi-tag
0x7ff282c7f050   144  lsl x4, x4, #32
                  ;;; <@48,#90> smi-untag
0x7ff282c7f054   148  asr x5, x4, #32
                  ;;; <@50,#31> mul-const-i-s
0x7ff282c7f058   152  lsl w6, w5, #3

BUG=
R=jkummerow@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21818 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/hydrogen-representation-changes.cc

index b916d9b..6cca536 100644 (file)
@@ -51,6 +51,15 @@ void HRepresentationChangesPhase::InsertRepresentationChangeForUse(
 }
 
 
+static bool IsNonDeoptingIntToSmiChange(HChange* change) {
+  Representation from_rep = change->from();
+  Representation to_rep = change->to();
+  // Flags indicating Uint32 operations are set in a later Hydrogen phase.
+  ASSERT(!change->CheckFlag(HValue::kUint32));
+  return from_rep.IsInteger32() && to_rep.IsSmi() && SmiValuesAre32Bits();
+}
+
+
 void HRepresentationChangesPhase::InsertRepresentationChangesForValue(
     HValue* value) {
   Representation r = value->representation();
@@ -65,17 +74,33 @@ 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. If
+    // that's int32, and this HForceRepresentation use is int32, and int32 to
+    // smi changes can't cause deoptimisation, set the input of the use to the
+    // input of the HChange.
+    if (value->IsForceRepresentation()) {
+      HValue* input = HForceRepresentation::cast(value)->value();
+      if (input->IsChange()) {
+        HChange* change = HChange::cast(input);
+        if (change->from().Equals(req) && IsNonDeoptingIntToSmiChange(change)) {
+          use_value->SetOperandAt(use_index, change->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());
+    }
   }
 }