[x64] Improve key value sign-extension of dehoisted LoadKeyed/StoreKeyed

src/x64/lithium-x64.cc

Index: src/x64/lithium-x64.cc
diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index d42d3824e2f37d2b9ee94888e0d4c8bbcc8eb286..8a09537b92cd5b4af3960785768e23c7e8ae2b6d 100644
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -175,6 +175,18 @@ bool LGoto::HasInterestingComment(LCodeGen* gen) const {
 }
 
 
+template<int R>
+bool LTemplateResultInstruction<R>::MustSignExtendResult(
+    LPlatformChunk* chunk) const {
+  HValue* hvalue = this->hydrogen_value();
+
+  if (hvalue == NULL) return false;
+  if (hvalue->HasRange() && !hvalue->range()->CanBeNegative()) return false;
+
+  return chunk->GetKeyValues()->Contains(hvalue->id());
+}
+
+
 void LGoto::PrintDataTo(StringStream* stream) {
   stream->Add("B%d", block_id());
 }
@@ -1964,12 +1976,35 @@ LInstruction* LChunkBuilder::DoLoadRoot(HLoadRoot* instr) {
 }
 
 
+void RecordKeyValueThroughPhi(HValue* phi, BitVector* key_values) {

[danno, 2014/03/18 08:20:20]

This should be called FindDehoistedKeyDefinitions, make it a private member of
the ChunkBuilder so that you don't have to pass in the BitVector.

void LChunkBuilder::FindDehoistedKeyDefinitions(HValue* camdidate) {
  if (dehoisted_key_ids_->Contains(camdidate->id())) return;
  dehoisted_key_ids_->Add(value->id());
  if (!candidate->IsPhi()) return;
  for (int i = 0; i < candidate->OperandCount(); ++i) {
    FindDehoistedKeyDefinitions(candidate->OperandAt(i));
  }
}

[Weiliang, 2014/03/19 08:55:31]

The BitVector belongs to LChunk instead of LChunkBuilder. After making this
function private, BitVector is accessed through chunk_->GetDehoistedKeyIds()
Done.

+  ASSERT(phi->IsPhi());
+  for (int i = 0; i < phi->OperandCount(); ++i) {
+    HValue* value = phi->OperandAt(i);
+    if (key_values->Contains(value->id())) continue;
+    key_values->Add(value->id());
+    if (value->IsPhi()) {
+      RecordKeyValueThroughPhi(value, key_values);
+    }
+  }
+}
+
+
 LInstruction* LChunkBuilder::DoLoadKeyed(HLoadKeyed* instr) {
   ASSERT(instr->key()->representation().IsInteger32());
   ElementsKind elements_kind = instr->elements_kind();
   LOperand* key = UseRegisterOrConstantAtStart(instr->key());
   LLoadKeyed* result = NULL;
 
+  if (instr->IsDehoisted()) {
+    BitVector* key_values = chunk()->GetKeyValues();
+    if (!key_values->Contains(instr->key()->id())) {

[danno, 2014/03/18 08:20:20]

Move most of the logic here into RecordKeyValueThroughPhi. The code here should
be very simple:

if (instr->IsDehoisted()) {
  FindDehoistedKeyDefinitions(instr->key());
}

[Weiliang, 2014/03/19 08:55:31]

Done.

+      key_values->Add(instr->key()->id());
+      if (instr->key()->IsPhi()) {
+        RecordKeyValueThroughPhi(instr->key(), key_values);
+      }
+    }
+  }
+
   if (!instr->is_typed_elements()) {
     LOperand* obj = UseRegisterAtStart(instr->elements());
     result = new(zone()) LLoadKeyed(obj, key);
@@ -2007,6 +2042,16 @@ LInstruction* LChunkBuilder::DoLoadKeyedGeneric(HLoadKeyedGeneric* instr) {
 LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) {
   ElementsKind elements_kind = instr->elements_kind();
 
+  if (instr->IsDehoisted()) {
+    BitVector* key_values = chunk()->GetKeyValues();

[danno, 2014/03/18 08:20:20]

See comment above.

[Weiliang, 2014/03/19 08:55:31]

Done.

+    if (!key_values->Contains(instr->key()->id())) {
+      key_values->Add(instr->key()->id());
+      if (instr->key()->IsPhi()) {
+        RecordKeyValueThroughPhi(instr->key(), key_values);
+      }
+    }
+  }
+
   if (!instr->is_typed_elements()) {
     ASSERT(instr->elements()->representation().IsTagged());
     bool needs_write_barrier = instr->NeedsWriteBarrier();
@@ -2017,7 +2062,7 @@ LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) {
     Representation value_representation = instr->value()->representation();
     if (value_representation.IsDouble()) {
       object = UseRegisterAtStart(instr->elements());
-      val = UseTempRegister(instr->value());
+      val = UseRegisterAtStart(instr->value());
       key = UseRegisterOrConstantAtStart(instr->key());
     } else {
       ASSERT(value_representation.IsSmiOrTagged() ||