[ic] Inline LoadIC into LdaNamedProperty bytecode handler

src/ic/accessor-assembler.cc

Index: src/ic/accessor-assembler.cc
diff --git a/src/ic/accessor-assembler.cc b/src/ic/accessor-assembler.cc
index 5645124ccdfa622305fa6eab89f28700f06d83b2..a3e11cd8f447d965e991b4b6bb68f2bccabd23e9 100644
--- a/src/ic/accessor-assembler.cc
+++ b/src/ic/accessor-assembler.cc
@@ -19,14 +19,42 @@ using compiler::Node;
 
 //////////////////// Private helpers.
 
+Node* AccessorAssembler::LoadFeedbackSlot(Node* vector, Node* slot,

[jgruber, 2017/03/03 13:31:14]

This is basically LoadFixedArrayElement but with the header_size optimization as
commented below. Any reason we can't make that change in LFAE and use that
instead?

+                                          int additional_offset) {
+  int32_t header_size =
+      FixedArray::kHeaderSize - kHeapObjectTag + additional_offset;
+
+  // Adding |header_size| with a separate IntPtrAdd rather than passing it
+  // into ElementOffsetFromIndex() allows it to be folded into a single
+  // [base, index, offset] indirect memory access on x64.
+  Node* offset =
+      ElementOffsetFromIndex(slot, FAST_HOLEY_ELEMENTS, SMI_PARAMETERS);
+  return Load(MachineType::AnyTagged(), vector,
+              IntPtrAdd(offset, IntPtrConstant(header_size)));
+}
+
 Node* AccessorAssembler::TryMonomorphicCase(Node* slot, Node* vector,
                                             Node* receiver_map,
                                             Label* if_handler,
                                             Variable* var_handler,
                                             Label* if_miss) {
-  Comment("TryMonomorphicCase");
   DCHECK_EQ(MachineRepresentation::kTagged, var_handler->rep());
 
+  Node* feedback = nullptr;
+  Node* handler = nullptr;
+  std::tie(feedback, handler) =
+      TryMonomorphicCase(slot, vector, receiver_map, if_miss);
+
+  var_handler->Bind(handler);
+  Goto(if_handler);
+
+  return feedback;
+}
+
+std::pair<Node*, Node*> AccessorAssembler::TryMonomorphicCase(

[jgruber, 2017/03/03 13:31:14]

This version avoids needs a variable and a label less. It's used in
LdaNamedProperty.

+    Node* slot, Node* vector, Node* receiver_map, Label* if_miss) {
+  Comment("TryMonomorphicCase");
+
   // TODO(ishell): add helper class that hides offset computations for a series
   // of loads.
   int32_t header_size = FixedArray::kHeaderSize - kHeapObjectTag;
@@ -48,9 +76,7 @@ Node* AccessorAssembler::TryMonomorphicCase(Node* slot, Node* vector,
       Load(MachineType::AnyTagged(), vector,
            IntPtrAdd(offset, IntPtrConstant(header_size + kPointerSize)));
 
-  var_handler->Bind(handler);
-  Goto(if_handler);
-  return feedback;
+  return std::make_pair(feedback, handler);
 }
 
 void AccessorAssembler::HandlePolymorphicCase(Node* receiver_map,
@@ -141,9 +167,8 @@ void AccessorAssembler::HandleKeyedStorePolymorphicCase(
 
 void AccessorAssembler::HandleLoadICHandlerCase(
     const LoadICParameters* p, Node* handler, Label* miss,
-    ElementSupport support_elements) {
+    ExitPoint* exit_point, ElementSupport support_elements) {
   Comment("have_handler");
-  ExitPoint direct_exit(this);
 
   Variable var_holder(this, MachineRepresentation::kTagged, p->receiver);
   Variable var_smi_handler(this, MachineRepresentation::kTagged, handler);
@@ -160,21 +185,21 @@ void AccessorAssembler::HandleLoadICHandlerCase(
   Bind(&if_smi_handler);
   {
     HandleLoadICSmiHandlerCase(p, var_holder.value(), var_smi_handler.value(),
-                               miss, &direct_exit, support_elements);
+                               miss, exit_point, support_elements);
   }
 
   Bind(&try_proto_handler);
   {
     GotoIf(IsCodeMap(LoadMap(handler)), &call_handler);
     HandleLoadICProtoHandlerCase(p, handler, &var_holder, &var_smi_handler,
-                                 &if_smi_handler, miss, &direct_exit, false);
+                                 &if_smi_handler, miss, exit_point, false);
   }
 
   Bind(&call_handler);
   {
     typedef LoadWithVectorDescriptor Descriptor;
-    TailCallStub(Descriptor(isolate()), handler, p->context, p->receiver,
-                 p->name, p->slot, p->vector);
+    exit_point->ReturnCallStub(Descriptor(isolate()), handler, p->context,
+                               p->receiver, p->name, p->slot, p->vector);
   }
 }
 
@@ -273,8 +298,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
     Comment("property_load");
   }
 
-  Label constant(this, Label::kDeferred), field(this),
-      normal(this, Label::kDeferred);
+  Label constant(this), field(this), normal(this, Label::kDeferred);

[jgruber, 2017/03/03 13:31:14]

Both constant and field cases can omit frame construction like this.

   GotoIf(WordEqual(handler_kind, IntPtrConstant(LoadHandler::kForFields)),
          &field);
 
@@ -664,7 +688,7 @@ void AccessorAssembler::HandleStoreICProtoHandler(const StoreICParameters* p,
     Node* transition = var_transition.value();
     Node* handler_word = SmiUntag(smi_handler);
 
-    GotoIf(IsSetWord32<Map::Deprecated>(LoadMapBitField3(transition)), miss);
+    GotoIf(IsDeprecatedMap(transition), miss);
 
     Node* handler_kind = DecodeWord<StoreHandler::KindBits>(handler_word);
     GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kStoreNormal)),
@@ -1322,6 +1346,8 @@ void AccessorAssembler::GenericPropertyLoad(Node* receiver, Node* receiver_map,
                                             const LoadICParameters* p,
                                             Label* slow,
                                             UseStubCache use_stub_cache) {
+  ExitPoint direct_exit(this);
+
   Comment("key is unique name");
   Label if_found_on_receiver(this), if_property_dictionary(this),
       lookup_prototype_chain(this);
@@ -1368,7 +1394,7 @@ void AccessorAssembler::GenericPropertyLoad(Node* receiver, Node* receiver_map,
     TryProbeStubCache(isolate()->load_stub_cache(), receiver, key,
                       &found_handler, &var_handler, &stub_cache_miss);
     Bind(&found_handler);
-    { HandleLoadICHandlerCase(p, var_handler.value(), slow); }
+    { HandleLoadICHandlerCase(p, var_handler.value(), slow, &direct_exit); }
 
     Bind(&stub_cache_miss);
     {
@@ -1577,29 +1603,55 @@ void AccessorAssembler::TryProbeStubCache(StubCache* stub_cache, Node* receiver,
 //////////////////// Entry points into private implementation (one per stub).
 
 void AccessorAssembler::LoadIC(const LoadICParameters* p) {
+  // Must be kept in sync with Interpreter::BuildLoadIC.
+
+  ExitPoint direct_exit(this);
+
   Variable var_handler(this, MachineRepresentation::kTagged);
-  Label if_handler(this, &var_handler), try_polymorphic(this, Label::kDeferred),
-      try_megamorphic(this, Label::kDeferred),
-      try_uninitialized(this, Label::kDeferred), miss(this, Label::kDeferred);
+  Label if_handler(this, &var_handler), non_monomorphic(this, Label::kDeferred),
+      miss(this, Label::kDeferred);
 
   Node* receiver_map = LoadReceiverMap(p->receiver);
-  GotoIf(IsSetWord32<Map::Deprecated>(LoadMapBitField3(receiver_map)), &miss);
+  GotoIf(IsDeprecatedMap(receiver_map), &miss);
 
   // Check monomorphic case.
   Node* feedback =
       TryMonomorphicCase(p->slot, p->vector, receiver_map, &if_handler,
-                         &var_handler, &try_polymorphic);
+                         &var_handler, &non_monomorphic);
   Bind(&if_handler);
-  { HandleLoadICHandlerCase(p, var_handler.value(), &miss); }
+  HandleLoadICHandlerCase(p, var_handler.value(), &miss, &direct_exit);
+
+  Bind(&non_monomorphic);
+  LoadIC_Noninlined(p, receiver_map, feedback, &var_handler, &if_handler, &miss,
+                    &direct_exit);
+
+  Bind(&miss);
+  direct_exit.ReturnCallRuntime(Runtime::kLoadIC_Miss, p->context, p->receiver,
+                                p->name, p->slot, p->vector);
+}
+
+void AccessorAssembler::LoadIC_Noninlined(const LoadICParameters* p,

[jgruber, 2017/03/03 13:31:14]

I don't have a better name for this. Right now it's 'non-monomorphic,
non-deprecated map' but that can change as we inline more things.

+                                          Node* receiver_map, Node* feedback,
+                                          Variable* var_handler,
+                                          Label* if_handler, Label* miss,
+                                          ExitPoint* exit_point) {
+  Label try_megamorphic(this, Label::kDeferred),
+      try_uninitialized(this, Label::kDeferred);
+
+  // Neither deprecated map nor monomorphic. These cases are handled in the
+  // bytecode handler.
+  CSA_ASSERT(this, Word32BinaryNot(IsDeprecatedMap(receiver_map)));
+  CSA_ASSERT(this,
+             WordNotEqual(receiver_map, LoadWeakCellValueUnchecked(feedback)));
+  DCHECK_EQ(MachineRepresentation::kTagged, var_handler->rep());
 
-  Bind(&try_polymorphic);
   {
     // Check polymorphic case.
     Comment("LoadIC_try_polymorphic");
     GotoIfNot(WordEqual(LoadMap(feedback), FixedArrayMapConstant()),
               &try_megamorphic);
-    HandlePolymorphicCase(receiver_map, feedback, &if_handler, &var_handler,
-                          &miss, 2);
+    HandlePolymorphicCase(receiver_map, feedback, if_handler, var_handler, miss,
+                          2);
   }
 
   Bind(&try_megamorphic);
@@ -1609,21 +1661,18 @@ void AccessorAssembler::LoadIC(const LoadICParameters* p) {
               &try_uninitialized);
 
     TryProbeStubCache(isolate()->load_stub_cache(), p->receiver, p->name,
-                      &if_handler, &var_handler, &miss);
+                      if_handler, var_handler, miss);
   }
+
   Bind(&try_uninitialized);
   {
     // Check uninitialized case.
     GotoIfNot(
         WordEqual(feedback, LoadRoot(Heap::kuninitialized_symbolRootIndex)),
-        &miss);
-    TailCallStub(CodeFactory::LoadIC_Uninitialized(isolate()), p->context,
-                 p->receiver, p->name, p->slot, p->vector);
-  }
-  Bind(&miss);
-  {
-    TailCallRuntime(Runtime::kLoadIC_Miss, p->context, p->receiver, p->name,
-                    p->slot, p->vector);
+        miss);
+    exit_point->ReturnCallStub(CodeFactory::LoadIC_Uninitialized(isolate()),
+                               p->context, p->receiver, p->name, p->slot,
+                               p->vector);
   }
 }
 
@@ -1767,6 +1816,8 @@ void AccessorAssembler::LoadGlobalIC_MissCase(const LoadICParameters* p,
 
 void AccessorAssembler::LoadGlobalIC(const LoadICParameters* p,
                                      TypeofMode typeof_mode) {
+  // Must be kept in sync with Interpreter::BuildLoadGlobal.
+
   ExitPoint direct_exit(this);
 
   Label try_handler(this), miss(this);
@@ -1781,6 +1832,8 @@ void AccessorAssembler::LoadGlobalIC(const LoadICParameters* p,
 }
 
 void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p) {
+  ExitPoint direct_exit(this);
+
   Variable var_handler(this, MachineRepresentation::kTagged);
   Label if_handler(this, &var_handler), try_polymorphic(this, Label::kDeferred),
       try_megamorphic(this, Label::kDeferred),
@@ -1788,14 +1841,17 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p) {
       miss(this, Label::kDeferred);
 
   Node* receiver_map = LoadReceiverMap(p->receiver);
-  GotoIf(IsSetWord32<Map::Deprecated>(LoadMapBitField3(receiver_map)), &miss);
+  GotoIf(IsDeprecatedMap(receiver_map), &miss);
 
   // Check monomorphic case.
   Node* feedback =
       TryMonomorphicCase(p->slot, p->vector, receiver_map, &if_handler,
                          &var_handler, &try_polymorphic);
   Bind(&if_handler);
-  { HandleLoadICHandlerCase(p, var_handler.value(), &miss, kSupportElements); }
+  {
+    HandleLoadICHandlerCase(p, var_handler.value(), &miss, &direct_exit,
+                            kSupportElements);
+  }
 
   Bind(&try_polymorphic);
   {
@@ -1824,10 +1880,7 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p) {
     GotoIfNot(WordEqual(feedback, p->name), &miss);
     // If the name comparison succeeded, we know we have a fixed array with
     // at least one map/handler pair.
-    Node* offset = ElementOffsetFromIndex(
-        p->slot, FAST_HOLEY_ELEMENTS, SMI_PARAMETERS,
-        FixedArray::kHeaderSize + kPointerSize - kHeapObjectTag);
-    Node* array = Load(MachineType::AnyTagged(), p->vector, offset);
+    Node* array = LoadFeedbackSlot(p->vector, p->slot, kPointerSize);
     HandlePolymorphicCase(receiver_map, array, &if_handler, &var_handler, &miss,
                           1);
   }
@@ -1881,7 +1934,7 @@ void AccessorAssembler::StoreIC(const StoreICParameters* p) {
       try_megamorphic(this, Label::kDeferred), miss(this, Label::kDeferred);
 
   Node* receiver_map = LoadReceiverMap(p->receiver);
-  GotoIf(IsSetWord32<Map::Deprecated>(LoadMapBitField3(receiver_map)), &miss);
+  GotoIf(IsDeprecatedMap(receiver_map), &miss);
 
   // Check monomorphic case.
   Node* feedback =
@@ -1932,7 +1985,7 @@ void AccessorAssembler::KeyedStoreIC(const StoreICParameters* p,
         try_polymorphic_name(this, Label::kDeferred);
 
     Node* receiver_map = LoadReceiverMap(p->receiver);
-    GotoIf(IsSetWord32<Map::Deprecated>(LoadMapBitField3(receiver_map)), &miss);
+    GotoIf(IsDeprecatedMap(receiver_map), &miss);
 
     // Check monomorphic case.
     Node* feedback =
@@ -2013,10 +2066,7 @@ void AccessorAssembler::KeyedStoreIC(const StoreICParameters* p,
       GotoIfNot(WordEqual(feedback, p->name), &miss);
       // If the name comparison succeeded, we know we have a FixedArray with
       // at least one map/handler pair.
-      Node* offset = ElementOffsetFromIndex(
-          p->slot, FAST_HOLEY_ELEMENTS, SMI_PARAMETERS,
-          FixedArray::kHeaderSize + kPointerSize - kHeapObjectTag);
-      Node* array = Load(MachineType::AnyTagged(), p->vector, offset);
+      Node* array = LoadFeedbackSlot(p->vector, p->slot, kPointerSize);
       HandlePolymorphicCase(receiver_map, array, &if_handler, &var_handler,
                             &miss, 1);
     }
@@ -2044,6 +2094,34 @@ void AccessorAssembler::GenerateLoadIC() {
   LoadIC(&p);
 }
 
+void AccessorAssembler::GenerateLoadIC_Noninlined() {
+  typedef LoadWithVectorDescriptor Descriptor;
+
+  Node* receiver = Parameter(Descriptor::kReceiver);
+  Node* name = Parameter(Descriptor::kName);
+  Node* slot = Parameter(Descriptor::kSlot);
+  Node* vector = Parameter(Descriptor::kVector);
+  Node* context = Parameter(Descriptor::kContext);
+
+  ExitPoint direct_exit(this);
+  Variable var_handler(this, MachineRepresentation::kTagged);
+  Label if_handler(this, &var_handler), miss(this, Label::kDeferred);
+
+  Node* receiver_map = LoadReceiverMap(receiver);
+  Node* feedback = LoadFeedbackSlot(vector, slot, 0);

[jgruber, 2017/03/03 13:31:14]

We could avoid these two loads if we pass the map and feedback to the stub.
Don't think it makes a big difference though.

+
+  LoadICParameters p(context, receiver, name, slot, vector);
+  LoadIC_Noninlined(&p, receiver_map, feedback, &var_handler, &if_handler,
+                    &miss, &direct_exit);
+
+  Bind(&if_handler);
+  HandleLoadICHandlerCase(&p, var_handler.value(), &miss, &direct_exit);
+
+  Bind(&miss);
+  direct_exit.ReturnCallRuntime(Runtime::kLoadIC_Miss, context, receiver, name,
+                                slot, vector);
+}
+
 void AccessorAssembler::GenerateLoadIC_Uninitialized() {
   typedef LoadWithVectorDescriptor Descriptor;