Revert of VectorICs: ia32 store ics need a virtual register. (patchset #3 id:40001...
authormachenbach <machenbach@chromium.org>
Mon, 14 Sep 2015 18:58:49 +0000 (11:58 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 14 Sep 2015 18:59:01 +0000 (18:59 +0000)
Reason for revert:
[Sheriff] Breaks GC stress on mac:
http://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/2536

Original issue's description:
> VectorICs: ia32 store ics need a virtual register.
>
> 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=
>
> Committed: https://crrev.com/b26e98f19b5e77df50cb4a259f099ee6afa335e3
> Cr-Commit-Position: refs/heads/master@{#30718}

TBR=jkummerow@chromium.org,vogelheim@chromium.org,mvstanton@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

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

src/assembler.cc
src/assembler.h
src/flag-definitions.h
src/ia32/code-stubs-ia32.cc
src/ic/ia32/stub-cache-ia32.cc
src/isolate.cc
src/isolate.h
src/snapshot/serialize.cc

index c81cdc2..49b300e 100644 (file)
@@ -1407,12 +1407,6 @@ 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) {
index 433b9b8..10e9bb4 100644 (file)
@@ -990,8 +990,6 @@ 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.
index 1de7203..850208b 100644 (file)
@@ -697,7 +697,7 @@ DEFINE_BOOL(use_idle_notification, true,
 // ic.cc
 DEFINE_BOOL(use_ic, true, "use inline caching")
 DEFINE_BOOL(trace_ic, false, "trace inline cache state transitions")
-DEFINE_BOOL(vector_stores, true, "use vectors for store ics")
+DEFINE_BOOL(vector_stores, false, "use vectors for store ics")
 DEFINE_BOOL(global_var_shortcuts, true, "use ic-less global loads and stores")
 
 // macro-assembler-ia32.cc
index 4fcdabc..c685d22 100644 (file)
@@ -4567,8 +4567,6 @@ 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);
@@ -4595,9 +4593,8 @@ static void HandlePolymorphicStoreCase(MacroAssembler* masm, Register receiver,
   __ pop(vector);
   __ pop(receiver);
   __ lea(handler, FieldOperand(handler, Code::kHeaderSize));
-  __ mov(Operand::StaticVariable(virtual_register), handler);
-  __ pop(handler);  // Pop "value".
-  __ jmp(Operand::StaticVariable(virtual_register));
+  __ xchg(handler, Operand(esp, 0));
+  __ ret(0);
 
   // Polymorphic, we have to loop from 2 to N
 
@@ -4617,13 +4614,12 @@ 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);
-  __ mov(Operand::StaticVariable(virtual_register), handler);
-  __ pop(handler);  // Pop "value".
-  __ jmp(Operand::StaticVariable(virtual_register));
+  __ lea(handler, FieldOperand(handler, Code::kHeaderSize));
+  __ xchg(handler, Operand(esp, 0));
+  __ ret(0);
 
   __ bind(&prepare_next);
   __ add(counter, Immediate(Smi::FromInt(2)));
@@ -4648,8 +4644,6 @@ 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;
@@ -4666,10 +4660,9 @@ 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.
-  __ mov(Operand::StaticVariable(virtual_register), weak_cell);
-  __ pop(weak_cell);  // Pop "value".
-  // jump to the handler.
-  __ jmp(Operand::StaticVariable(virtual_register));
+  __ xchg(weak_cell, Operand(esp, 0));
+  // "return" to the handler.
+  __ ret(0);
 
   // 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.
@@ -4679,10 +4672,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));
-  __ mov(Operand::StaticVariable(virtual_register), weak_cell);
-  __ pop(weak_cell);  // Pop "value".
-  // jump to the handler.
-  __ jmp(Operand::StaticVariable(virtual_register));
+  // Put the store ic value back in it's register.
+  __ xchg(weak_cell, Operand(esp, 0));
+  // "return" to the handler.
+  __ ret(0);
 }
 
 
@@ -4755,8 +4748,6 @@ 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);
@@ -4793,9 +4784,8 @@ static void HandlePolymorphicKeyedStoreCase(MacroAssembler* masm,
   __ pop(vector);
   __ pop(receiver);
   __ lea(feedback, FieldOperand(feedback, Code::kHeaderSize));
-  __ mov(Operand::StaticVariable(virtual_register), feedback);
-  __ pop(feedback);  // Pop "value".
-  __ jmp(Operand::StaticVariable(virtual_register));
+  __ xchg(feedback, Operand(esp, 0));
+  __ ret(0);
 
   __ bind(&transition_call);
   // Oh holy hell this will be tough.
index 7366ebe..e579179 100644 (file)
@@ -23,8 +23,6 @@ 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 =
@@ -69,10 +67,9 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm,
       DCHECK(extra.is(VectorStoreICDescriptor::SlotRegister()));
       __ add(extra, Immediate(Code::kHeaderSize - kHeapObjectTag));
       __ pop(vector);
-      __ mov(Operand::StaticVariable(virtual_register), extra);
-      __ pop(extra);  // Pop "slot".
+      __ xchg(extra, Operand(esp, 0));
       // Jump to the first instruction in the code stub.
-      __ jmp(Operand::StaticVariable(virtual_register));
+      __ ret(0);
     } else {
       __ pop(LoadWithVectorDescriptor::VectorRegister());
       __ pop(LoadDescriptor::SlotRegister());
@@ -127,10 +124,9 @@ 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);
-      __ pop(offset);  // Pop "slot".
-      __ jmp(Operand::StaticVariable(virtual_register));
+      __ xchg(offset, Operand(esp, 0));
+      __ ret(0);
     } else {
       __ add(offset, Immediate(Code::kHeaderSize - kHeapObjectTag));
       __ jmp(offset);
index 3d57fb1..b8b3c26 100644 (file)
@@ -1765,7 +1765,6 @@ 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),
index 035b4b3..b9ee070 100644 (file)
@@ -1024,10 +1024,6 @@ 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.
@@ -1303,8 +1299,6 @@ 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
index 2d52053..4b3284e 100644 (file)
@@ -129,8 +129,6 @@ 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(),