Prevent deopt when assigning double values to typed arrays

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index f6c47f31dbe83934de00a230eb9099fc12dcacf0..cc069c894201d4fc583bb7bbcdae9e785be70ec4 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -919,7 +919,7 @@ void HRangeAnalysis::InferControlFlowRange(HTest* test, HBasicBlock* dest) {
   ASSERT((test->FirstSuccessor() == dest) == (test->SecondSuccessor() != dest));
   if (test->value()->IsCompare()) {
     HCompare* compare = HCompare::cast(test->value());
-    if (compare->GetInputRepresentation().IsInteger32()) {
+    if (compare->GetInputRepresentation().IsInteger()) {
       Token::Value op = compare->token();
       if (test->SecondSuccessor() == dest) {
         op = Token::NegateCompareOp(op);
@@ -1505,7 +1505,7 @@ class HInferRepresentation BASE_EMBEDDED {
 
 
 void HInferRepresentation::AddToWorklist(HValue* current) {
-  if (current->representation().IsSpecialization()) return;
+  if (current->representation().IsTerminalSpecialization()) return;
   if (!current->CheckFlag(HValue::kFlexibleRepresentation)) return;
   if (in_worklist_.Contains(current->id())) return;
   worklist_.Add(current);
@@ -1519,10 +1519,10 @@ void HInferRepresentation::AddToWorklist(HValue* current) {
 // becomes the new representation type of the node.
 void HInferRepresentation::InferBasedOnInputs(HValue* current) {
   Representation r = current->representation();
-  if (r.IsSpecialization()) return;
+  if (r.IsTerminalSpecialization()) return;
   ASSERT(current->CheckFlag(HValue::kFlexibleRepresentation));
   Representation inferred = current->InferredRepresentation();
-  if (inferred.IsSpecialization()) {
+  if (!inferred.Equals(r)) {
     current->ChangeRepresentation(inferred);
     AddDependantsToWorklist(current);
   }
@@ -1545,7 +1545,7 @@ void HInferRepresentation::AddDependantsToWorklist(HValue* current) {
 // specialized.
 void HInferRepresentation::InferBasedOnUses(HValue* current) {
   Representation r = current->representation();
-  if (r.IsSpecialization() || current->HasNoUses()) return;
+  if (r.IsTerminalSpecialization() || current->HasNoUses()) return;
   ASSERT(current->CheckFlag(HValue::kFlexibleRepresentation));
   Representation new_rep = TryChange(current);
   if (!new_rep.IsNone()) {
@@ -1578,7 +1578,12 @@ Representation HInferRepresentation::TryChange(HValue* current) {
   int tagged_count = use_count[Representation::kTagged];
   int double_count = use_count[Representation::kDouble];
   int int32_count = use_count[Representation::kInteger32];
-  int non_tagged_count = double_count + int32_count;
+  int trunc_int32_count = use_count[Representation::kTruncatedInteger32];
+  int clamped_rounded_int8_count =
+      use_count[Representation::kClampedRoundedUInteger8];
+  int non_tagged_count = double_count + int32_count +
+      trunc_int32_count + clamped_rounded_int8_count;
+  int total_use_count = tagged_count + non_tagged_count;
 
   // If a non-loop phi has tagged uses, don't convert it to untagged.
   if (current->IsPhi() && !current->block()->IsLoopHeader()) {
@@ -1593,6 +1598,14 @@ Representation HInferRepresentation::TryChange(HValue* current) {
       return Representation::Double();
     } else if (int32_count > 0) {
       return Representation::Integer32();
+    } else if (clamped_rounded_int8_count > 0 &&
+               clamped_rounded_int8_count == total_use_count) {
+        return Representation::ClampedRoundedUInteger8();
+    } else if (trunc_int32_count > 0 &&
+               trunc_int32_count == total_use_count) {
+        return Representation::TruncatedInteger32();
+    } else {
+      return Representation::None();
     }
   }
   return Representation::None();
@@ -1764,18 +1777,28 @@ void HGraph::InsertRepresentationChangeForUse(HValue* value,
   // information we treat constants like normal instructions and insert the
   // change instructions for them.
   HInstruction* new_value = NULL;
-  bool is_truncating = use->CheckFlag(HValue::kTruncatingToInt32);
   if (value->IsConstant()) {
     HConstant* constant = HConstant::cast(value);
     // Try to create a new copy of the constant with the new representation.
-    new_value = is_truncating
+    new_value = to.IsTruncatedInteger32()
         ? constant->CopyToTruncatedInt32()
-        : constant->CopyToRepresentation(to);
+        : to.IsClampedRoundedInteger8()
+            ? constant->CopyToClampedRoundedUInt8()
+            : constant->CopyToRepresentation(to);
   }
 
   if (new_value == NULL) {
+    Representation from = value->representation();
+    // Don't create changes for representation differences that
+    // won't generate code.
+    if (from.IsInteger() && (to.IsInteger32X() || to.IsTruncatedInteger32())) {
+      return;
+    }
+
     new_value =
-        new(zone()) HChange(value, value->representation(), to, is_truncating);
+        new(zone()) HChange(value,
+                            from,
+                            to);
   }
 
   new_value->InsertBefore(next);
@@ -1783,25 +1806,6 @@ void HGraph::InsertRepresentationChangeForUse(HValue* value,
 }
 
 
-int CompareConversionUses(HValue* a,
-                          HValue* b,
-                          Representation a_rep,
-                          Representation b_rep) {
-  if (a_rep.kind() > b_rep.kind()) {
-    // Make sure specializations are separated in the result array.
-    return 1;
-  }
-  // Put truncating conversions before non-truncating conversions.
-  bool a_truncate = a->CheckFlag(HValue::kTruncatingToInt32);
-  bool b_truncate = b->CheckFlag(HValue::kTruncatingToInt32);
-  if (a_truncate != b_truncate) {
-    return a_truncate ? -1 : 1;
-  }
-  // Sort by increasing block ID.
-  return a->block()->block_id() - b->block()->block_id();
-}
-
-
 void HGraph::InsertRepresentationChangesForValue(
     HValue* current,
     ZoneList<HValue*>* to_convert,
@@ -1814,6 +1818,7 @@ void HGraph::InsertRepresentationChangesForValue(
   // us to avoid duplicate changes without searching the list.
   ASSERT(to_convert->is_empty());
   ASSERT(to_convert_reps->is_empty());
+  int index = 0;
   for (int i = 0; i < current->uses()->length(); ++i) {
     HValue* use = current->uses()->at(i);
     // The occurrences index means the index within the operand array of "use"
@@ -1832,27 +1837,20 @@ void HGraph::InsertRepresentationChangesForValue(
     int operand_index = use->LookupOperandIndex(occurrence_index, current);
     Representation req = use->RequiredInputRepresentation(operand_index);
     if (req.IsNone() || req.Equals(r)) continue;
-    int index = 0;
-    while (index < to_convert->length() &&
-           CompareConversionUses(to_convert->at(index),

[fschneider, 2011/05/10 10:44:53]

Thanks for cleaning this up!

-                                 use,
-                                 to_convert_reps->at(index),
-                                 req) < 0) {
-      ++index;
-    }
-    if (FLAG_trace_representation) {
-      PrintF("Inserting a representation change to %s of %d for use at %d\n",
-             req.Mnemonic(),
-             current->id(),
-             use->id());
-    }
     to_convert->InsertAt(index, use);
     to_convert_reps->InsertAt(index, req);
+    ++index;
   }
 
   for (int i = 0; i < to_convert->length(); ++i) {
     HValue* use = to_convert->at(i);
     Representation r_to = to_convert_reps->at(i);
+    if (FLAG_trace_representation) {
+      PrintF("Inserting a representation change to %s of %d for use at %d\n",
+             r_to.Mnemonic(),
+             current->id(),
+             use->id());
+    }
     InsertRepresentationChangeForUse(current, use, r_to);
   }
 
@@ -1867,36 +1865,6 @@ void HGraph::InsertRepresentationChangesForValue(
 
 void HGraph::InsertRepresentationChanges() {
   HPhase phase("Insert representation changes", this);

[fschneider, 2011/05/10 10:44:53]

Not sure that the representation computation does the right thing. Why can you
just remove this part?

Maybe we can do something a little simpler without introducing completely
separate representations for ClampedUint8 and TruncatedInt32.

All values for ClampedUint8, int8, int16, int32, uint8 and uint16 can be
precisely represented as our current Representation::Integer32.

How about attaching the additional information to the range-information and let
the HStoreKeyedSpecialized decide whether to generate conversion code based on
the range and the representation of the input value.

This would avoid the complications with having phi-instructions and phi-uses of
even more different representations.

We still have a small problem with Uint32 values since they are outside the
int32 range - but probably we should deal with this one in a separate change
anyway.

-
-
-  // Compute truncation flag for phis: Initially assume that all
-  // int32-phis allow truncation and iteratively remove the ones that
-  // are used in an operation that does not allow a truncating
-  // conversion.
-  // TODO(fschneider): Replace this with a worklist-based iteration.
-  for (int i = 0; i < phi_list()->length(); i++) {
-    HPhi* phi = phi_list()->at(i);
-    if (phi->representation().IsInteger32()) {
-      phi->SetFlag(HValue::kTruncatingToInt32);
-    }
-  }
-  bool change = true;
-  while (change) {
-    change = false;
-    for (int i = 0; i < phi_list()->length(); i++) {
-      HPhi* phi = phi_list()->at(i);
-      if (!phi->CheckFlag(HValue::kTruncatingToInt32)) continue;
-      for (int j = 0; j < phi->uses()->length(); j++) {
-        HValue* use = phi->uses()->at(j);
-        if (!use->CheckFlag(HValue::kTruncatingToInt32)) {
-          phi->ClearFlag(HValue::kTruncatingToInt32);
-          change = true;
-          break;
-        }
-      }
-    }
-  }
-
   ZoneList<HValue*> value_list(4);
   ZoneList<Representation> rep_list(4);
   for (int i = 0; i < blocks_.length(); ++i) {
@@ -1928,7 +1896,7 @@ void HGraph::ComputeMinusZeroChecks() {
         // int32-to-tagged and int32-to-double.
         Representation from = change->value()->representation();
         ASSERT(from.Equals(change->from()));
-        if (from.IsInteger32()) {
+        if (from.IsInteger() && !change->to().IsInteger()) {
           ASSERT(change->to().IsTagged() || change->to().IsDouble());
           ASSERT(visited.IsEmpty());
           PropagateMinusZeroChecks(change->value(), &visited);