Consolidated all the key store/load classes in the Hydrogen and Lithium

src/arm/lithium-arm.cc

Index: src/arm/lithium-arm.cc
diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index 21c549f175fbbe83c5b1b7e81d2ddba2146f9a11..0b83777a5560cc7762e52c58707464be89d384d8 100644
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -372,7 +372,7 @@ void LStoreNamedGeneric::PrintDataTo(StringStream* stream) {
 }
 
 
-void LStoreKeyedFastElement::PrintDataTo(StringStream* stream) {
+void LStoreKeyed::PrintDataTo(StringStream* stream) {
   object()->PrintTo(stream);
   stream->Add("[");
   key()->PrintTo(stream);
@@ -381,15 +381,6 @@ void LStoreKeyedFastElement::PrintDataTo(StringStream* stream) {
 }
 
 
-void LStoreKeyedFastDoubleElement::PrintDataTo(StringStream* stream) {
-  elements()->PrintTo(stream);
-  stream->Add("[");
-  key()->PrintTo(stream);
-  stream->Add("] <- ");
-  value()->PrintTo(stream);
-}
-
-
 void LStoreKeyedGeneric::PrintDataTo(StringStream* stream) {
   object()->PrintTo(stream);
   stream->Add("[");
@@ -1860,34 +1851,31 @@ LInstruction* LChunkBuilder::DoLoadExternalArrayPointer(
 }
 
 
-LInstruction* LChunkBuilder::DoLoadKeyedFastElement(
-    HLoadKeyedFastElement* instr) {
-  ASSERT(instr->representation().IsTagged());
+LInstruction* LChunkBuilder::DoLoadKeyed(
+    HLoadKeyed* instr) {
   ASSERT(instr->key()->representation().IsInteger32() ||
          instr->key()->representation().IsTagged());
-  LOperand* obj = UseRegisterAtStart(instr->object());
-  LOperand* key = UseRegisterOrConstantAtStart(instr->key());
-  LLoadKeyedFastElement* result = new(zone()) LLoadKeyedFastElement(obj, key);
-  if (instr->RequiresHoleCheck()) AssignEnvironment(result);
-  return DefineAsRegister(result);
-}
 
+  if (!instr->is_external()) {
+    LOperand* key = UseRegisterOrConstantAtStart(instr->key());
+    LOperand* obj = NULL;
+    if (instr->representation().IsDouble()) {
+      obj = UseTempRegister(instr->object());
+    } else {
+      ASSERT(instr->representation().IsTagged());
+      obj = UseRegisterAtStart(instr->object());
+    }
 
-LInstruction* LChunkBuilder::DoLoadKeyedFastDoubleElement(
-    HLoadKeyedFastDoubleElement* instr) {
-  ASSERT(instr->representation().IsDouble());
-  ASSERT(instr->key()->representation().IsInteger32() ||
-         instr->key()->representation().IsTagged());
-  LOperand* elements = UseTempRegister(instr->elements());
-  LOperand* key = UseRegisterOrConstantAtStart(instr->key());
-  LLoadKeyedFastDoubleElement* result =
-      new(zone()) LLoadKeyedFastDoubleElement(elements, key);
-  return AssignEnvironment(DefineAsRegister(result));
-}
+    LLoadKeyed* result = new(zone()) LLoadKeyed(obj, key);
 
+    // TODO(mvstanton): looks like we always called AssignEnvironment
+    // for the FastDouble
+    // case, even if it didn't need a hole check (packed double case).
+    // I've quit doing that...is that okay?
+    if (instr->RequiresHoleCheck()) AssignEnvironment(result);

[danno, 2012/10/21 20:44:41]

make sure that AssignEnvironment is the last thing done for consistency. It
should operate on the result of DefineAsRegister(result).

[mvstanton, 2012/10/23 23:44:20]

Done.

+    return DefineAsRegister(result);
+  }
 
-LInstruction* LChunkBuilder::DoLoadKeyedSpecializedArrayElement(
-    HLoadKeyedSpecializedArrayElement* instr) {
   ElementsKind elements_kind = instr->elements_kind();
   ASSERT(
       (instr->representation().IsInteger32() &&
@@ -1896,12 +1884,10 @@ LInstruction* LChunkBuilder::DoLoadKeyedSpecializedArrayElement(
       (instr->representation().IsDouble() &&
        ((elements_kind == EXTERNAL_FLOAT_ELEMENTS) ||
        (elements_kind == EXTERNAL_DOUBLE_ELEMENTS))));
-  ASSERT(instr->key()->representation().IsInteger32() ||
-         instr->key()->representation().IsTagged());
   LOperand* external_pointer = UseRegister(instr->external_pointer());
   LOperand* key = UseRegisterOrConstant(instr->key());

[danno, 2012/10/21 20:44:41]

This can be UseRegisterOrConstantAtStart, I think, too, so you can combine the
key calculate. UseRegisterOrConstantAtStart just means that it can re-use the
register passed in for the key as the output register, if I recall correctly,
UseRegisterOrConstant doesn't. In the arm case (and other platforms, too), the
load result and the key aren't live at the same time for any of the load
variants, so it's OK to use UseRegisterOrConstantAtStart.

[mvstanton, 2012/10/23 23:44:20]

Done.

-  LLoadKeyedSpecializedArrayElement* result =
-      new(zone()) LLoadKeyedSpecializedArrayElement(external_pointer, key);
+  LLoadKeyed* result =
+      new(zone()) LLoadKeyed(external_pointer, key);
   LInstruction* load_instr = DefineAsRegister(result);
   // An unsigned int array load might overflow and cause a deopt, make sure it
   // has an environment.
@@ -1920,66 +1906,55 @@ LInstruction* LChunkBuilder::DoLoadKeyedGeneric(HLoadKeyedGeneric* instr) {
 }
 
 
-LInstruction* LChunkBuilder::DoStoreKeyedFastElement(
-    HStoreKeyedFastElement* instr) {
-  bool needs_write_barrier = instr->NeedsWriteBarrier();
-  ASSERT(instr->value()->representation().IsTagged());
-  ASSERT(instr->object()->representation().IsTagged());
-  ASSERT(instr->key()->representation().IsInteger32() ||
-         instr->key()->representation().IsTagged());
-
-  LOperand* obj = UseTempRegister(instr->object());
-  LOperand* val = needs_write_barrier
-      ? UseTempRegister(instr->value())
-      : UseRegisterAtStart(instr->value());
-  LOperand* key = needs_write_barrier
-      ? UseTempRegister(instr->key())
-      : UseRegisterOrConstantAtStart(instr->key());
-  return new(zone()) LStoreKeyedFastElement(obj, key, val);
-}
-
-
-LInstruction* LChunkBuilder::DoStoreKeyedFastDoubleElement(
-    HStoreKeyedFastDoubleElement* instr) {
-  ASSERT(instr->value()->representation().IsDouble());
-  ASSERT(instr->elements()->representation().IsTagged());
+LInstruction* LChunkBuilder::DoStoreKeyed(
+    HStoreKeyed* instr) {
   ASSERT(instr->key()->representation().IsInteger32() ||
          instr->key()->representation().IsTagged());
 
-  LOperand* elements = UseRegisterAtStart(instr->elements());
-  LOperand* val = UseTempRegister(instr->value());
-  LOperand* key = UseRegisterOrConstantAtStart(instr->key());
-
-  return new(zone()) LStoreKeyedFastDoubleElement(elements, key, val);
-}
+  if (!instr->is_external()) {
+    ASSERT(instr->object()->representation().IsTagged());
 
+    if (instr->value()->representation().IsDouble()) {
+      LOperand* object = UseRegisterAtStart(instr->object());
+      LOperand* val = UseTempRegister(instr->value());
+      LOperand* key = UseRegisterOrConstantAtStart(instr->key());
 
-LInstruction* LChunkBuilder::DoStoreKeyedSpecializedArrayElement(
-    HStoreKeyedSpecializedArrayElement* instr) {
-  ElementsKind elements_kind = instr->elements_kind();
-  ASSERT(
-      (instr->value()->representation().IsInteger32() &&
-       (elements_kind != EXTERNAL_FLOAT_ELEMENTS) &&
-       (elements_kind != EXTERNAL_DOUBLE_ELEMENTS)) ||
-      (instr->value()->representation().IsDouble() &&
-       ((elements_kind == EXTERNAL_FLOAT_ELEMENTS) ||
-       (elements_kind == EXTERNAL_DOUBLE_ELEMENTS))));
-  ASSERT(instr->external_pointer()->representation().IsExternal());
-  ASSERT(instr->key()->representation().IsInteger32() ||
-         instr->key()->representation().IsTagged());
-
-  LOperand* external_pointer = UseRegister(instr->external_pointer());
-  bool val_is_temp_register =
-      elements_kind == EXTERNAL_PIXEL_ELEMENTS ||
-      elements_kind == EXTERNAL_FLOAT_ELEMENTS;
-  LOperand* val = val_is_temp_register
-      ? UseTempRegister(instr->value())
-      : UseRegister(instr->value());
-  LOperand* key = UseRegisterOrConstant(instr->key());
-
-  return new(zone()) LStoreKeyedSpecializedArrayElement(external_pointer,
-                                                        key,
-                                                        val);
+      return new(zone()) LStoreKeyed(object, key, val);
+    } else {
+      bool needs_write_barrier = instr->NeedsWriteBarrier();
+      ASSERT(instr->value()->representation().IsTagged());
+
+      LOperand* obj = UseTempRegister(instr->object());
+      LOperand* val = needs_write_barrier
+          ? UseTempRegister(instr->value())
+          : UseRegisterAtStart(instr->value());
+      LOperand* key = needs_write_barrier
+          ? UseTempRegister(instr->key())
+          : UseRegisterOrConstantAtStart(instr->key());
+      return new(zone()) LStoreKeyed(obj, key, val);
+    }
+  } else {
+    ElementsKind elements_kind = instr->elements_kind();
+    ASSERT(
+        (instr->value()->representation().IsInteger32() &&
+         (elements_kind != EXTERNAL_FLOAT_ELEMENTS) &&
+         (elements_kind != EXTERNAL_DOUBLE_ELEMENTS)) ||
+        (instr->value()->representation().IsDouble() &&
+         ((elements_kind == EXTERNAL_FLOAT_ELEMENTS) ||
+          (elements_kind == EXTERNAL_DOUBLE_ELEMENTS))));
+    ASSERT(instr->external_pointer()->representation().IsExternal());
+
+    LOperand* external_pointer = UseRegister(instr->external_pointer());
+    bool val_is_temp_register =
+        elements_kind == EXTERNAL_PIXEL_ELEMENTS ||
+        elements_kind == EXTERNAL_FLOAT_ELEMENTS;
+    LOperand* val = val_is_temp_register
+        ? UseTempRegister(instr->value())
+        : UseRegister(instr->value());

[danno, 2012/10/21 20:44:41]

Use the previous four lines to compute val in both the external and non-external
case. in the non external case, elements_kind won't be EXTERNAL_PIXEL_ELEMENTS
or EXTERNAL_FLOAT_ELEMENTS, so you'll get a temporary register as expected.

[mvstanton, 2012/10/23 23:44:20]

Done, but there was a pretty big variation in the ia32 tree that meant I
couldn't refactor as deeply there. Have a look at that change coming up and see
what you think.

+    LOperand* key = UseRegisterOrConstant(instr->key());

[danno, 2012/10/21 20:44:41]

As in load case, use LOperand* key = UseRegisterOrConstantAtStart(instr->key());
for both branches.

[mvstanton, 2012/10/23 23:44:20]

Done.

+
+    return new(zone()) LStoreKeyed(external_pointer, key, val);
+  }
 }