Improve constant element index access code generation

src/arm/lithium-codegen-arm.cc

Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index d94a1feb238c5e0abef2c1d2f670e30e2c72c6b9..90a315b2b6e360e1e6951ced76147b2ceff92f39 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -2779,20 +2779,30 @@ void LCodeGen::DoAccessArgumentsAt(LAccessArgumentsAt* instr) {
 
 void LCodeGen::DoLoadKeyedFastElement(LLoadKeyedFastElement* instr) {
   Register elements = ToRegister(instr->elements());
-  Register key = EmitLoadRegister(instr->key(), scratch0());
   Register result = ToRegister(instr->result());
   Register scratch = scratch0();
 
-  // Load the result.
-  if (instr->hydrogen()->key()->representation().IsTagged()) {
-    __ add(scratch, elements,
-           Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize));
+  if (instr->key()->IsConstantOperand()) {
+    LConstantOperand* const_operand = LConstantOperand::cast(instr->key());
+    int offset =
+        (ToInteger32(const_operand) + instr->additional_index()) * kPointerSize
+        + FixedArray::kHeaderSize;
+    __ ldr(result, FieldMemOperand(elements, offset));
   } else {
-    __ add(scratch, elements, Operand(key, LSL, kPointerSizeLog2));
+    Register key = EmitLoadRegister(instr->key(), scratch0());
+    if (instr->hydrogen()->key()->representation().IsTagged()) {

[Jakob Kummerow, 2012/07/30 11:58:08]

I don't see how this condition can ever be true. HLoadKeyedFastElements forces
Integer32 representation for its |key| input.

[danno, 2012/07/30 16:11:11]

Actually, this does get triggered. It's scary. The HLoadKeyedFastElement
instruction depends on the BoundsCheck instruction. The representation type of
the bounds check isn't important, but it has to be consistent with the index
input to the load. During the phase that eliminates redundant bounds checks, the
key input to the load is replaced by the input to the boundscheck, which can be
tagged. Adding a comment to this effect.

On 2012/07/30 11:58:08, Jakob wrote:

+      __ add(scratch, elements,
+             Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize));
+    } else {
+      __ add(scratch, elements, Operand(key, LSL, kPointerSizeLog2));
+    }
+    if (instr->additional_index() != 0) {
+      __ add(scratch,
+             scratch,
+             Operand(instr->additional_index() << kPointerSizeLog2));

[Jakob Kummerow, 2012/07/30 11:58:08]

What's the benefit of having this addition in generated code here?
instr->additional_index() is a constant; adding it to FixedArray::kHeaderSize at
Hydrogen runtime seems fine to me.

[danno, 2012/07/30 16:11:11]

I changed this here and in the DoStoreKeyedFastElement code to be consistent (I
copied it here from there originally to make sure the cases were handled
identically).
On 2012/07/30 11:58:08, Jakob wrote:
Done.

+    }
+    __ ldr(result, FieldMemOperand(scratch, FixedArray::kHeaderSize));
   }
-  uint32_t offset = FixedArray::kHeaderSize +
-                    (instr->additional_index() << kPointerSizeLog2);
-  __ ldr(result, FieldMemOperand(scratch, offset));
 
   // Check for the hole value.
   if (instr->hydrogen()->RequiresHoleCheck()) {
@@ -3823,7 +3833,18 @@ void LCodeGen::DoStoreNamedGeneric(LStoreNamedGeneric* instr) {
 
 
 void LCodeGen::DoBoundsCheck(LBoundsCheck* instr) {
-  __ cmp(ToRegister(instr->index()), ToRegister(instr->length()));
+  if (instr->index()->IsConstantOperand()) {
+    int constant_index =
+        ToInteger32(LConstantOperand::cast(instr->index()));
+    if (instr->hydrogen()->length()->representation().IsTagged()) {
+      __ mov(ip, Operand(Smi::FromInt(constant_index)));
+    } else {
+      __ mov(ip, Operand(constant_index));
+    }
+    __ cmp(ip, ToRegister(instr->length()));
+  } else {
+    __ cmp(ToRegister(instr->index()), ToRegister(instr->length()));
+  }
   DeoptimizeIf(hs, instr->environment());
 }