Elements kind conversion in generated code (ia32).

src/ia32/code-stubs-ia32.cc

Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index 1e886e202b315f03ca34e28b7749ba8c9a81f834..59e125a09c96bd880fe09fff289c72dd23d48ced 100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -34,6 +34,7 @@
 #include "isolate.h"
 #include "jsregexp.h"
 #include "regexp-macro-assembler.h"
+#include "stub-cache.h"
 
 namespace v8 {
 namespace internal {
@@ -6749,6 +6750,12 @@ struct AheadOfTimeWriteBarrierStubList kAheadOfTime[] = {
   { ebx, edx, ecx, EMIT_REMEMBERED_SET},
   // KeyedStoreStubCompiler::GenerateStoreFastElement.
   { edi, edx, ecx, EMIT_REMEMBERED_SET},
+  // FastElementConversionStub::GenerateSmiOnlyToObject
+  // and FastElementsConversionStub::GenerateSmiOnlyToDouble
+  // and FastElementsConversionStub::GenerateDoubleToObject
+  { edx, ebx, edi, EMIT_REMEMBERED_SET},
+  // FastElementConversionStub::GenerateDoubleToObject
+  { eax, edx, esi, EMIT_REMEMBERED_SET},
   // Null termination.
   { no_reg, no_reg, no_reg, EMIT_REMEMBERED_SET}
 };
@@ -6992,6 +6999,251 @@ void RecordWriteStub::CheckNeedsToInformIncrementalMarker(
 }
 
 
+void FastElementsConversionStub::GenerateSmiOnlyToObject(MacroAssembler* masm) {
+  // ----------- S t a t e -------------
+  //  -- eax    : value
+  //  -- ebx    : target map
+  //  -- ecx    : key
+  //  -- edx    : receiver
+  //  -- esp[0] : return address
+  // -----------------------------------
+  // Set transitioned map.
+  __ mov(FieldOperand(edx, HeapObject::kMapOffset), ebx);
+  __ RecordWriteField(edx,
+                      HeapObject::kMapOffset,
+                      ebx,
+                      edi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+}
+
+
+void FastElementsConversionStub::GenerateSmiOnlyToDouble(
+    MacroAssembler* masm, StrictModeFlag strict_mode) {
+  // ----------- S t a t e -------------
+  //  -- eax    : value
+  //  -- ebx    : target map
+  //  -- ecx    : key
+  //  -- edx    : receiver
+  //  -- esp[0] : return address
+  // -----------------------------------
+  Label loop, entry, convert_hole, gc_required;
+  __ push(eax);
+  __ push(ebx);
+
+  __ mov(esi, FieldOperand(edx, JSObject::kElementsOffset));
+  __ mov(esi, FieldOperand(esi, FixedArray::kLengthOffset));
+
+  // Allocate new FixedDoubleArray.
+  // edx: receiver
+  // esi: length of source FixedArray (smi-tagged)
+  __ lea(edi, Operand(esi, times_4, FixedDoubleArray::kHeaderSize));
+  __ AllocateInNewSpace(edi, eax, ebx, no_reg, &gc_required, TAG_OBJECT);
+
+  // eax: destination FixedDoubleArray
+  // esi: number of elements
+  // edx: receiver
+  // edi: source FixedArray

[Jakob Kummerow, 2011/10/13 09:15:59]

Please move this comment to some place where the statement is true.

+  __ mov(FieldOperand(eax, HeapObject::kMapOffset),
+         Immediate(masm->isolate()->factory()->fixed_double_array_map()));
+  __ mov(FieldOperand(eax, FixedDoubleArray::kLengthOffset), esi);
+  // Replace receiver's backing store with newly created FixedDoubleArray.
+  __ mov(ebx, eax);
+  __ RecordWriteField(edx,
+                      JSObject::kElementsOffset,
+                      ebx,
+                      edi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+  __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset));
+  // Write barrier has been notified earlier.
+  __ mov(FieldOperand(edx, JSObject::kElementsOffset), eax);
+
+  // Convert and copy elements
+  // edi: source FixedArray

[Jakob Kummerow, 2011/10/13 09:15:59]

You can omit all comments for registers whose contents have not changed since
the last comment. Up to you.

[Yang, 2011/10/13 09:58:53]

Done.

+  // eax: destination FixedDoubleArray
+  // esi: number of elements to convert/copy
+  // edx: receiver
+  ExternalReference canonical_the_hole_nan_reference =
+      ExternalReference::address_of_the_hole_nan();
+  XMMRegister the_hole_nan = xmm1;
+  if (CpuFeatures::IsSupported(SSE2)) {
+    CpuFeatures::Scope use_sse2(SSE2);
+    __ movdbl(the_hole_nan,
+              Operand::StaticVariable(canonical_the_hole_nan_reference));
+  }
+  __ jmp(&entry);
+  __ bind(&loop);
+  __ sub(esi, Immediate(Smi::FromInt(1)));
+  __ mov(ebx, FieldOperand(edi, esi, times_2, FixedArray::kHeaderSize));
+  // ebx: current element from source
+  // esi: index of current element
+  __ JumpIfNotSmi(ebx, &convert_hole);
+
+  // Normal smi, convert it to double and store.
+  __ SmiUntag(ebx);
+  if (CpuFeatures::IsSupported(SSE2)) {
+    CpuFeatures::Scope fscope(SSE2);
+    __ cvtsi2sd(xmm0, ebx);
+    __ movdbl(FieldOperand(eax, esi, times_4, FixedDoubleArray::kHeaderSize),
+              xmm0);
+  } else {
+    __ push(ebx);
+    __ fild_s(Operand(esp, 0));
+    __ pop(ebx);
+    __ fstp_d(FieldOperand(eax, esi, times_4, FixedDoubleArray::kHeaderSize));
+  }
+  __ jmp(&entry);
+
+  // Found hole, store hole_nan_as_double instead.
+  __ bind(&convert_hole);
+  if (CpuFeatures::IsSupported(SSE2)) {
+    CpuFeatures::Scope use_sse2(SSE2);
+    __ movdbl(FieldOperand(eax, esi, times_4, FixedDoubleArray::kHeaderSize),
+              the_hole_nan);
+  } else {
+    __ fld_d(Operand::StaticVariable(canonical_the_hole_nan_reference));
+    __ fstp_d(FieldOperand(eax, esi, times_4, FixedDoubleArray::kHeaderSize));
+  }
+
+  __ bind(&entry);
+  __ test(esi, esi);
+  __ j(not_zero, &loop);
+
+  // Restore necessary values do store.

[Jakob Kummerow, 2011/10/13 09:15:59]

I don't understand this comment.

[Yang, 2011/10/13 09:58:53]

Removed and replaced by comments below.

+  Label done;
+  __ pop(ebx);
+  __ pop(eax);
+  // Set transitioned map.
+  __ mov(FieldOperand(edx, HeapObject::kMapOffset), ebx);
+  __ RecordWriteField(edx,
+                      HeapObject::kMapOffset,
+                      ebx,
+                      edi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+  __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  __ jmp(&done, Label::kNear);
+
+  __ bind(&gc_required);
+  // Restore esp and esi before jumping into runtime.

[Jakob Kummerow, 2011/10/13 09:15:59]

Slightly outdated. s/esp and esi/registers/?

[Yang, 2011/10/13 09:58:53]

Done.

+  __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  __ pop(ebx);
+  __ pop(eax);
+  KeyedStoreIC::GenerateRuntimeSetProperty(masm, strict_mode);
+  __ bind(&done);
+}
+
+
+void FastElementsConversionStub::GenerateDoubleToObject(

[Jakob Kummerow, 2011/10/13 09:15:59]

TODO(jkummerow): continue reviewing here.

+    MacroAssembler* masm, StrictModeFlag strict_mode) {
+  // ----------- S t a t e -------------
+  //  -- eax    : value
+  //  -- ebx    : target map
+  //  -- ecx    : key
+  //  -- edx    : receiver
+  //  -- esp[0] : return address
+  // -----------------------------------
+  Label loop, entry, convert_hole, gc_required;
+  __ push(eax);
+  __ push(edx);
+  __ push(ebx);
+
+  __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset));
+  __ mov(ebx, FieldOperand(edi, FixedDoubleArray::kLengthOffset));
+
+  // Allocate new FixedArray.
+  // edx: receiver

[Jakob Kummerow, 2011/10/13 09:15:59]

Again, you can omit comments about registers that haven't changed.

+  // ebx: length of source FixedDoubleArray (smi-tagged)
+  __ lea(edi, Operand(ebx, times_2, FixedArray::kHeaderSize));
+  __ AllocateInNewSpace(edi, eax, esi, no_reg, &gc_required, TAG_OBJECT);
+
+  // eax: destination FixedDoubleArray

[Jakob Kummerow, 2011/10/13 09:15:59]

s/FixedDoubleArray/FixedArray/

+  // ebx: number of elements
+  // edx: receiver
+  // edi: source FixedArray

[Jakob Kummerow, 2011/10/13 09:15:59]

Please move this to where it's true. Also, s/FixedArray/FixedDoubleArray/. Or
just delete this comment here since you have the correct version four lines
below.

+  __ mov(FieldOperand(eax, HeapObject::kMapOffset),
+         Immediate(masm->isolate()->factory()->fixed_array_map()));
+  __ mov(FieldOperand(eax, FixedArray::kLengthOffset), ebx);
+  __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset));
+
+  // Box doubles into heap numbers.
+  // edi: source FixedDoubleArray
+  // eax: destination FixedArray
+  // ebx: index of current element (smi-tagged)

[Jakob Kummerow, 2011/10/13 09:15:59]

Move this comment.

+  __ jmp(&entry);
+  __ bind(&loop);
+  __ sub(ebx, Immediate(Smi::FromInt(1)));
+  uint32_t offset = FixedDoubleArray::kHeaderSize + sizeof(kHoleNanLower32);
+  __ cmp(FieldOperand(edi, ebx, times_4, offset), Immediate(kHoleNanUpper32));
+  __ j(equal, &convert_hole);
+
+  // Non-hole double, copy value into a heap number.
+  __ AllocateHeapNumber(edx, esi, no_reg, &gc_required);
+  // edx: new heap number
+  __ mov(esi, FieldOperand(edi, ebx, times_4, FixedDoubleArray::kHeaderSize));

[Jakob Kummerow, 2011/10/13 09:15:59]

You could check if SSE2 is available and if yes, do a single 64bit move using an
xmm register, falling back to two 32bit moves via esi otherwise.

[Yang, 2011/10/13 09:58:53]

Done.

+  __ mov(FieldOperand(edx, HeapNumber::kValueOffset), esi);
+  __ mov(esi, FieldOperand(edi, ebx, times_4, offset));
+  __ mov(FieldOperand(edx, HeapNumber::kValueOffset + kPointerSize), esi);
+  __ mov(FieldOperand(eax, ebx, times_2, FixedArray::kHeaderSize), edx);
+  __ mov(esi, ebx);
+  __ RecordWriteArray(eax,
+                      edx,
+                      esi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+  __ jmp(&entry);
+
+  // Replace the-hole nan with the-hole pointer.
+  __ bind(&convert_hole);
+  __ mov(FieldOperand(eax, ebx, times_2, FixedArray::kHeaderSize),
+         masm->isolate()->factory()->the_hole_value());
+
+  __ bind(&entry);
+  __ test(ebx, ebx);
+  __ j(not_zero, &loop);
+
+  // Replace receiver's backing store with newly created and filled FixedArray.
+  __ pop(ebx);
+  __ pop(edx);
+  // Set transitioned map.
+  __ mov(FieldOperand(edx, HeapObject::kMapOffset), ebx);
+  __ RecordWriteField(edx,
+                      HeapObject::kMapOffset,
+                      ebx,
+                      edi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+  __ mov(FieldOperand(edx, JSObject::kElementsOffset), eax);
+  __ mov(ebx, eax);

[Jakob Kummerow, 2011/10/13 09:15:59]

This move is unnecessary. EAX is popped next anyway, so it's OK for it to be
clobbered.

[Yang, 2011/10/13 09:58:53]

Done.

+  __ RecordWriteField(edx,
+                      JSObject::kElementsOffset,
+                      ebx,
+                      edi,
+                      kDontSaveFPRegs,
+                      EMIT_REMEMBERED_SET,
+                      OMIT_SMI_CHECK);
+
+  // Restore registers.
+  Label done;
+  __ pop(eax);
+  __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  __ jmp(&done, Label::kNear);
+
+  __ bind(&gc_required);
+  __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  __ pop(ebx);
+  __ pop(edx);
+  __ pop(eax);
+  KeyedStoreIC::GenerateRuntimeSetProperty(masm, strict_mode);
+  __ bind(&done);
+}
+
 #undef __
 
 } }  // namespace v8::internal