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

src/arm/lithium-arm.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 354 matching lines @@
 
 void LStoreNamedGeneric::PrintDataTo(StringStream* stream) {
   object()->PrintTo(stream);
   stream->Add(".");
   stream->Add(*String::cast(*name())->ToCString());
   stream->Add(" <- ");
   value()->PrintTo(stream);
 }
 
 
-void LStoreKeyedFastElement::PrintDataTo(StringStream* stream) {
+void LStoreKeyed::PrintDataTo(StringStream* stream) {
   object()->PrintTo(stream);
   stream->Add("[");
   key()->PrintTo(stream);
   stream->Add("] <- ");
   value()->PrintTo(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("[");
   key()->PrintTo(stream);
   stream->Add("] <- ");
   value()->PrintTo(stream);
 }
 
 
 void LTransitionElementsKind::PrintDataTo(StringStream* stream) {
@@ skipping 1450 matching lines @@
 }
 
 
 LInstruction* LChunkBuilder::DoLoadExternalArrayPointer(
     HLoadExternalArrayPointer* instr) {
   LOperand* input = UseRegisterAtStart(instr->value());
   return DefineAsRegister(new(zone()) LLoadExternalArrayPointer(input));
 }
 
 
-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() &&
        (elements_kind != EXTERNAL_FLOAT_ELEMENTS) &&
        (elements_kind != EXTERNAL_DOUBLE_ELEMENTS)) ||
       (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.
   return (elements_kind == EXTERNAL_UNSIGNED_INT_ELEMENTS) ?

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

You can combine this exit sequence with the one for non external elements:

bool can_deoptimize = instr->RequiresHoleCheck() || elements_kin ==
EXTERNAL_UNSIGNED_INT_ELEMENTS;
return can_deoptimize ? AssignEnvironment(result) : result;

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

Thanks, done. Indeed I could combine a lot more code in there, such as the key
creation.

       AssignEnvironment(load_instr) : load_instr;
 }
 
 
 LInstruction* LChunkBuilder::DoLoadKeyedGeneric(HLoadKeyedGeneric* instr) {
   LOperand* object = UseFixed(instr->object(), r1);
   LOperand* key = UseFixed(instr->key(), r0);
 
   LInstruction* result =
       DefineFixed(new(zone()) LLoadKeyedGeneric(object, key), r0);
   return MarkAsCall(result, instr);
 }
 
 
-LInstruction* LChunkBuilder::DoStoreKeyedFastElement(
-    HStoreKeyedFastElement* instr) {
-  bool needs_write_barrier = instr->NeedsWriteBarrier();
-  ASSERT(instr->value()->representation().IsTagged());
-  ASSERT(instr->object()->representation().IsTagged());
+LInstruction* LChunkBuilder::DoStoreKeyed(
+    HStoreKeyed* instr) {
   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);
+  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());
+
+      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);
+  }
 }
 
 
-LInstruction* LChunkBuilder::DoStoreKeyedFastDoubleElement(
-    HStoreKeyedFastDoubleElement* instr) {
-  ASSERT(instr->value()->representation().IsDouble());
-  ASSERT(instr->elements()->representation().IsTagged());
-  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);
-}
-
-
-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);
-}
-
-
 LInstruction* LChunkBuilder::DoStoreKeyedGeneric(HStoreKeyedGeneric* instr) {
   LOperand* obj = UseFixed(instr->object(), r2);
   LOperand* key = UseFixed(instr->key(), r1);
   LOperand* val = UseFixed(instr->value(), r0);
 
   ASSERT(instr->object()->representation().IsTagged());
   ASSERT(instr->key()->representation().IsTagged());
   ASSERT(instr->value()->representation().IsTagged());
 
   return MarkAsCall(new(zone()) LStoreKeyedGeneric(obj, key, val), instr);
@@ skipping 310 matching lines @@
 
 
 LInstruction* LChunkBuilder::DoLoadFieldByIndex(HLoadFieldByIndex* instr) {
   LOperand* object = UseRegister(instr->object());
   LOperand* index = UseRegister(instr->index());
   return DefineAsRegister(new(zone()) LLoadFieldByIndex(object, index));
 }
 
 
 } }  // namespace v8::internal