Reland VectorICs: ia32 store ics need a virtual register.

(reason for revert/reland: patch incorrectly left --vector-stores flag
 on, helpfully revealing some gcstress issues to look at, but they
 don't need to block this CL).

Some pretty hacky code was used to carry out the tail-call
handler dispatch on ia32 vector stores due to a lack
of free registers. It really tanks performance. A better
approach is to use a virtual register on the isolate.

BUG=
TBR=jkummerow@chromium.org, vogelheim@chromium.org

Review URL: https://codereview.chromium.org/1346573002

Cr-Commit-Position: refs/heads/master@{#30737}

diff --git a/src/assembler.cc b/src/assembler.cc
index 49b300e..c81cdc2 100644 (file)
--- a/src/assembler.cc
+++ b/src/assembler.cc
@@ -1407,6 +1407,12 @@ ExternalReference
 }
 
 
+ExternalReference ExternalReference::vector_store_virtual_register(
+    Isolate* isolate) {
+  return ExternalReference(isolate->vector_store_virtual_register_address());
+}
+
+
 double power_helper(double x, double y) {
   int y_int = static_cast<int>(y);
   if (y == y_int) {

diff --git a/src/assembler.h b/src/assembler.h
index 10e9bb4..433b9b8 100644 (file)
--- a/src/assembler.h
+++ b/src/assembler.h
@@ -990,6 +990,8 @@ class ExternalReference BASE_EMBEDDED {
   static ExternalReference invoke_function_callback(Isolate* isolate);
   static ExternalReference invoke_accessor_getter_callback(Isolate* isolate);
 
+  static ExternalReference vector_store_virtual_register(Isolate* isolate);
+
   Address address() const { return reinterpret_cast<Address>(address_); }
 
   // Used to check if single stepping is enabled in generated code.

diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index c685d22..4fcdabc 100644 (file)
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -4567,6 +4567,8 @@ static void HandlePolymorphicStoreCase(MacroAssembler* masm, Register receiver,
   Label next, next_loop, prepare_next;
   Label load_smi_map, compare_map;
   Label start_polymorphic;
+  ExternalReference virtual_register =
+      ExternalReference::vector_store_virtual_register(masm->isolate());
 
   __ push(receiver);
   __ push(vector);
@@ -4593,8 +4595,9 @@ static void HandlePolymorphicStoreCase(MacroAssembler* masm, Register receiver,
   __ pop(vector);
   __ pop(receiver);
   __ lea(handler, FieldOperand(handler, Code::kHeaderSize));
-  __ xchg(handler, Operand(esp, 0));
-  __ ret(0);
+  __ mov(Operand::StaticVariable(virtual_register), handler);
+  __ pop(handler);  // Pop "value".
+  __ jmp(Operand::StaticVariable(virtual_register));
 
   // Polymorphic, we have to loop from 2 to N
 
@@ -4614,12 +4617,13 @@ static void HandlePolymorphicStoreCase(MacroAssembler* masm, Register receiver,
   __ j(not_equal, &prepare_next);
   __ mov(handler, FieldOperand(feedback, counter, times_half_pointer_size,
                                FixedArray::kHeaderSize + kPointerSize));
+  __ lea(handler, FieldOperand(handler, Code::kHeaderSize));
   __ pop(key);
   __ pop(vector);
   __ pop(receiver);
-  __ lea(handler, FieldOperand(handler, Code::kHeaderSize));
-  __ xchg(handler, Operand(esp, 0));
-  __ ret(0);
+  __ mov(Operand::StaticVariable(virtual_register), handler);
+  __ pop(handler);  // Pop "value".
+  __ jmp(Operand::StaticVariable(virtual_register));
 
   __ bind(&prepare_next);
   __ add(counter, Immediate(Smi::FromInt(2)));
@@ -4644,6 +4648,8 @@ static void HandleMonomorphicStoreCase(MacroAssembler* masm, Register receiver,
                                        Label* miss) {
   // The store ic value is on the stack.
   DCHECK(weak_cell.is(VectorStoreICDescriptor::ValueRegister()));
+  ExternalReference virtual_register =
+      ExternalReference::vector_store_virtual_register(masm->isolate());
 
   // feedback initially contains the feedback array
   Label compare_smi_map;
@@ -4660,9 +4666,10 @@ static void HandleMonomorphicStoreCase(MacroAssembler* masm, Register receiver,
                                  FixedArray::kHeaderSize + kPointerSize));
   __ lea(weak_cell, FieldOperand(weak_cell, Code::kHeaderSize));
   // Put the store ic value back in it's register.
-  __ xchg(weak_cell, Operand(esp, 0));
-  // "return" to the handler.
-  __ ret(0);
+  __ mov(Operand::StaticVariable(virtual_register), weak_cell);
+  __ pop(weak_cell);  // Pop "value".
+  // jump to the handler.
+  __ jmp(Operand::StaticVariable(virtual_register));
 
   // In microbenchmarks, it made sense to unroll this code so that the call to
   // the handler is duplicated for a HeapObject receiver and a Smi receiver.
@@ -4672,10 +4679,10 @@ static void HandleMonomorphicStoreCase(MacroAssembler* masm, Register receiver,
   __ mov(weak_cell, FieldOperand(vector, slot, times_half_pointer_size,
                                  FixedArray::kHeaderSize + kPointerSize));
   __ lea(weak_cell, FieldOperand(weak_cell, Code::kHeaderSize));
-  // Put the store ic value back in it's register.
-  __ xchg(weak_cell, Operand(esp, 0));
-  // "return" to the handler.
-  __ ret(0);
+  __ mov(Operand::StaticVariable(virtual_register), weak_cell);
+  __ pop(weak_cell);  // Pop "value".
+  // jump to the handler.
+  __ jmp(Operand::StaticVariable(virtual_register));
 }
 
 
@@ -4748,6 +4755,8 @@ static void HandlePolymorphicKeyedStoreCase(MacroAssembler* masm,
   Label load_smi_map, compare_map;
   Label transition_call;
   Label pop_and_miss;
+  ExternalReference virtual_register =
+      ExternalReference::vector_store_virtual_register(masm->isolate());
 
   __ push(receiver);
   __ push(vector);
@@ -4784,8 +4793,9 @@ static void HandlePolymorphicKeyedStoreCase(MacroAssembler* masm,
   __ pop(vector);
   __ pop(receiver);
   __ lea(feedback, FieldOperand(feedback, Code::kHeaderSize));
-  __ xchg(feedback, Operand(esp, 0));
-  __ ret(0);
+  __ mov(Operand::StaticVariable(virtual_register), feedback);
+  __ pop(feedback);  // Pop "value".
+  __ jmp(Operand::StaticVariable(virtual_register));
 
   __ bind(&transition_call);
   // Oh holy hell this will be tough.

diff --git a/src/ic/ia32/stub-cache-ia32.cc b/src/ic/ia32/stub-cache-ia32.cc
index e579179..7366ebe 100644 (file)
--- a/src/ic/ia32/stub-cache-ia32.cc
+++ b/src/ic/ia32/stub-cache-ia32.cc
@@ -23,6 +23,8 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm,
   ExternalReference key_offset(isolate->stub_cache()->key_reference(table));
   ExternalReference value_offset(isolate->stub_cache()->value_reference(table));
   ExternalReference map_offset(isolate->stub_cache()->map_reference(table));
+  ExternalReference virtual_register =
+      ExternalReference::vector_store_virtual_register(masm->isolate());
 
   Label miss;
   bool is_vector_store =
@@ -67,9 +69,10 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm,
       DCHECK(extra.is(VectorStoreICDescriptor::SlotRegister()));
       __ add(extra, Immediate(Code::kHeaderSize - kHeapObjectTag));
       __ pop(vector);
-      __ xchg(extra, Operand(esp, 0));
+      __ mov(Operand::StaticVariable(virtual_register), extra);
+      __ pop(extra);  // Pop "slot".
       // Jump to the first instruction in the code stub.
-      __ ret(0);
+      __ jmp(Operand::StaticVariable(virtual_register));
     } else {
       __ pop(LoadWithVectorDescriptor::VectorRegister());
       __ pop(LoadDescriptor::SlotRegister());
@@ -124,9 +127,10 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm,
       Register vector = VectorStoreICDescriptor::VectorRegister();
       DCHECK(offset.is(VectorStoreICDescriptor::SlotRegister()));
       __ add(offset, Immediate(Code::kHeaderSize - kHeapObjectTag));
+      __ mov(Operand::StaticVariable(virtual_register), offset);
       __ pop(vector);
-      __ xchg(offset, Operand(esp, 0));
-      __ ret(0);
+      __ pop(offset);  // Pop "slot".
+      __ jmp(Operand::StaticVariable(virtual_register));
     } else {
       __ add(offset, Immediate(Code::kHeaderSize - kHeapObjectTag));
       __ jmp(offset);

diff --git a/src/isolate.cc b/src/isolate.cc
index b8b3c26..3d57fb1 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1765,6 +1765,7 @@ Isolate::Isolate(bool enable_serializer)
       deferred_handles_head_(NULL),
       optimizing_compile_dispatcher_(NULL),
       stress_deopt_count_(0),
+      vector_store_virtual_register_(NULL),
       next_optimization_id_(0),
 #if TRACE_MAPS
       next_unique_sfi_id_(0),

diff --git a/src/isolate.h b/src/isolate.h
index b9ee070..035b4b3 100644 (file)
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -1024,6 +1024,10 @@ class Isolate {
 
   void* stress_deopt_count_address() { return &stress_deopt_count_; }
 
+  void* vector_store_virtual_register_address() {
+    return &vector_store_virtual_register_;
+  }
+
   base::RandomNumberGenerator* random_number_generator();
 
   // Given an address occupied by a live code object, return that object.
@@ -1299,6 +1303,8 @@ class Isolate {
   // Counts deopt points if deopt_every_n_times is enabled.
   unsigned int stress_deopt_count_;
 
+  Address vector_store_virtual_register_;
+
   int next_optimization_id_;
 
 #if TRACE_MAPS

diff --git a/src/snapshot/serialize.cc b/src/snapshot/serialize.cc
index 4b3284e..2d52053 100644 (file)
--- a/src/snapshot/serialize.cc
+++ b/src/snapshot/serialize.cc
@@ -129,6 +129,8 @@ ExternalReferenceTable::ExternalReferenceTable(Isolate* isolate) {
       "double_constants.minus_one_half");
   Add(ExternalReference::stress_deopt_count(isolate).address(),
       "Isolate::stress_deopt_count_address()");
+  Add(ExternalReference::vector_store_virtual_register(isolate).address(),
+      "Isolate::vector_store_virtual_register()");
 
   // Debug addresses
   Add(ExternalReference::debug_after_break_target_address(isolate).address(),