Avoid using a register for constant external array indices.

src/arm/lithium-codegen-arm.cc

 // Copyright 2011 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 2458 matching lines @@
   // Check for the hole value.
   __ LoadRoot(scratch, Heap::kTheHoleValueRootIndex);
   __ cmp(result, scratch);
   DeoptimizeIf(eq, instr->environment());
 }
 
 
 void LCodeGen::DoLoadKeyedSpecializedArrayElement(
     LLoadKeyedSpecializedArrayElement* instr) {
   Register external_pointer = ToRegister(instr->external_pointer());
-  Register key = ToRegister(instr->key());
+  Register key = no_reg;
   ExternalArrayType array_type = instr->array_type();
+  bool key_is_constant = instr->key()->IsConstantOperand();
+  int constant_key = 0;
+  if (key_is_constant) {
+    constant_key = ToInteger32(LConstantOperand::cast(instr->key()));
+    if (constant_key & 0xF0000000) {
+      Abort("array index constant value too big.");
+    }
+  } else {
+    key = ToRegister(instr->key());
+  }
   if (array_type == kExternalFloatArray) {
     CpuFeatures::Scope scope(VFP3);
     DwVfpRegister result(ToDoubleRegister(instr->result()));
-    __ add(scratch0(), external_pointer, Operand(key, LSL, 2));
+    if (key_is_constant) {

[Jakob Kummerow, 2011/04/29 10:04:16]

Sure I can. Like this? 
(I don't really like duplicating the instruction code, but it's the second-most
concise way to do this.)

+      __ add(scratch0(), external_pointer, Operand(constant_key * 4));
+    } else {
+      __ add(scratch0(), external_pointer, Operand(key, LSL, 2));
+    }
     __ vldr(result.low(), scratch0(), 0);
     __ vcvt_f64_f32(result, result.low());
   } else if (array_type == kExternalDoubleArray) {
     CpuFeatures::Scope scope(VFP3);
     DwVfpRegister result(ToDoubleRegister(instr->result()));
-    __ add(scratch0(), external_pointer, Operand(key, LSL, 3));
+    Operand operand(key, LSL, 3);

[Jakob Kummerow, 2011/04/29 10:04:16]

Or like this?
(Empty initialization isn't possible, so I have to initialize for one case and
then overwrite for the other if necessary.)

+    if (key_is_constant) {
+      operand = Operand(constant_key * 8);
+    }
+    __ add(scratch0(), external_pointer, operand);
     __ vldr(result, scratch0(), 0);
   } else {
     Register result(ToRegister(instr->result()));
     switch (array_type) {
-      case kExternalByteArray:
-        __ ldrsb(result, MemOperand(external_pointer, key));
+      case kExternalByteArray: {

[Jakob Kummerow, 2011/04/29 10:04:16]

Or maybe like this?
(Doesn't avoid the multiline x?y:z construct, only moves it. Also, block scoping
with {} is necessary in this case.)

+        MemOperand operand(
+            key_is_constant ? MemOperand(external_pointer, constant_key)
+                            : MemOperand(external_pointer, key));
+        __ ldrsb(result, operand);
         break;
+      }
       case kExternalUnsignedByteArray:
       case kExternalPixelArray:
-        __ ldrb(result, MemOperand(external_pointer, key));
+        __ ldrb(result, key_is_constant

[Jakob Kummerow, 2011/04/29 10:04:16]

Or, on second thought, maybe stick with my original approach?

I can't quite decide which of these four solutions I find least ugly, so I went
for the most concise ugliness originally :-/

+                            ? MemOperand(external_pointer, constant_key)
+                            : MemOperand(external_pointer, key));
         break;
       case kExternalShortArray:
-        __ ldrsh(result, MemOperand(external_pointer, key, LSL, 1));
+        __ ldrsh(result, key_is_constant
+                             ? MemOperand(external_pointer, constant_key * 2)
+                             : MemOperand(external_pointer, key, LSL, 1));
         break;
       case kExternalUnsignedShortArray:
-        __ ldrh(result, MemOperand(external_pointer, key, LSL, 1));
+        __ ldrh(result, key_is_constant
+                            ? MemOperand(external_pointer, constant_key * 2)
+                            : MemOperand(external_pointer, key, LSL, 1));
         break;
       case kExternalIntArray:
-        __ ldr(result, MemOperand(external_pointer, key, LSL, 2));
+        __ ldr(result, key_is_constant
+                           ? MemOperand(external_pointer, constant_key * 4)
+                           : MemOperand(external_pointer, key, LSL, 2));
         break;
       case kExternalUnsignedIntArray:
-        __ ldr(result, MemOperand(external_pointer, key, LSL, 2));
+        __ ldr(result, key_is_constant
+                           ? MemOperand(external_pointer, constant_key * 4)
+                           : MemOperand(external_pointer, key, LSL, 2));
         __ cmp(result, Operand(0x80000000));
         // TODO(danno): we could be more clever here, perhaps having a special
         // version of the stub that detects if the overflow case actually
         // happens, and generate code that returns a double rather than int.
         DeoptimizeIf(cs, instr->environment());
         break;
       case kExternalFloatArray:
       case kExternalDoubleArray:
         UNREACHABLE();
         break;
@@ skipping 709 matching lines @@
     __ add(key, scratch, Operand(FixedArray::kHeaderSize));
     __ RecordWrite(elements, key, value);
   }
 }
 
 
 void LCodeGen::DoStoreKeyedSpecializedArrayElement(
     LStoreKeyedSpecializedArrayElement* instr) {
 
   Register external_pointer = ToRegister(instr->external_pointer());
-  Register key = ToRegister(instr->key());
+  Register key = no_reg;
   ExternalArrayType array_type = instr->array_type();
+  bool key_is_constant = instr->key()->IsConstantOperand();
+  int constant_key = 0;
+  if (key_is_constant) {
+    constant_key = ToInteger32(LConstantOperand::cast(instr->key()));
+    if (constant_key & 0xF0000000) {
+      Abort("array index constant value too big.");
+    }
+  } else {
+    key = ToRegister(instr->key());
+  }
 
   if (array_type == kExternalFloatArray) {
     CpuFeatures::Scope scope(VFP3);
     DwVfpRegister value(ToDoubleRegister(instr->value()));
-    __ add(scratch0(), external_pointer, Operand(key, LSL, 2));
+    __ add(scratch0(), external_pointer,
+           key_is_constant ? Operand(constant_key * 4) : Operand(key, LSL, 2));
     __ vcvt_f32_f64(double_scratch0().low(), value);
     __ vstr(double_scratch0().low(), scratch0(), 0);
   } else if (array_type == kExternalDoubleArray) {
     CpuFeatures::Scope scope(VFP3);
     DwVfpRegister value(ToDoubleRegister(instr->value()));
-    __ add(scratch0(), external_pointer, Operand(key, LSL, 3));
+    __ add(scratch0(), external_pointer,
+           key_is_constant ? Operand(constant_key * 8) : Operand(key, LSL, 3));
     __ vstr(value, scratch0(), 0);
   } else {
     Register value(ToRegister(instr->value()));
     switch (array_type) {
       case kExternalPixelArray:
         // Clamp the value to [0..255].
         __ Usat(value, 8, Operand(value));
-        __ strb(value, MemOperand(external_pointer, key));
+        __ strb(value, key_is_constant
+                           ? MemOperand(external_pointer, constant_key)
+                           : MemOperand(external_pointer, key));
         break;
       case kExternalByteArray:
       case kExternalUnsignedByteArray:
-        __ strb(value, MemOperand(external_pointer, key));
+        __ strb(value, key_is_constant
+                           ? MemOperand(external_pointer, constant_key)
+                           : MemOperand(external_pointer, key));
         break;
       case kExternalShortArray:
       case kExternalUnsignedShortArray:
-        __ strh(value, MemOperand(external_pointer, key, LSL, 1));
+        __ strh(value, key_is_constant
+                           ? MemOperand(external_pointer, constant_key * 2)
+                           : MemOperand(external_pointer, key, LSL, 1));
         break;
       case kExternalIntArray:
       case kExternalUnsignedIntArray:
-        __ str(value, MemOperand(external_pointer, key, LSL, 2));
+        __ str(value, key_is_constant
+                          ? MemOperand(external_pointer, constant_key * 4)
+                          : MemOperand(external_pointer, key, LSL, 2));
         break;
       case kExternalFloatArray:
       case kExternalDoubleArray:
         UNREACHABLE();
         break;
     }
   }
 }
 
 
@@ skipping 1018 matching lines @@
   ASSERT(osr_pc_offset_ == -1);
   osr_pc_offset_ = masm()->pc_offset();
 }
 
 
 
 
 #undef __
 
 } }  // namespace v8::internal