Improve constant element index access code generation

src/arm/lithium-codegen-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 2761 matching lines @@
 
   // There are two words between the frame pointer and the last argument.
   // Subtracting from length accounts for one of them add one more.
   __ add(length, length, Operand(1));
   __ ldr(result, MemOperand(arguments, length, LSL, kPointerSizeLog2));
 }
 
 
 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()) {
     if (IsFastSmiElementsKind(instr->hydrogen()->elements_kind())) {
       __ tst(result, Operand(kSmiTagMask));
       DeoptimizeIf(ne, instr->environment());
     } else {
       __ LoadRoot(scratch, Heap::kTheHoleValueRootIndex);
       __ cmp(result, scratch);
       DeoptimizeIf(eq, instr->environment());
@@ skipping 1010 matching lines @@
   // Name is always in r2.
   __ mov(r2, Operand(instr->name()));
   Handle<Code> ic = (instr->strict_mode_flag() == kStrictMode)
       ? isolate()->builtins()->StoreIC_Initialize_Strict()
       : isolate()->builtins()->StoreIC_Initialize();
   CallCode(ic, RelocInfo::CODE_TARGET, 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());
 }
 
 
 void LCodeGen::DoStoreKeyedFastElement(LStoreKeyedFastElement* instr) {
   Register value = ToRegister(instr->value());
   Register elements = ToRegister(instr->object());
   Register key = instr->key()->IsRegister() ? ToRegister(instr->key()) : no_reg;
   Register scratch = scratch0();
 
@@ skipping 1610 matching lines @@
   __ sub(scratch, result, Operand(index, LSL, kPointerSizeLog2 - kSmiTagSize));
   __ ldr(result, FieldMemOperand(scratch,
                                  FixedArray::kHeaderSize - kPointerSize));
   __ bind(&done);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal