[ic] Inline LoadGlobalIC in bytecode handlers

src/interpreter/interpreter.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/interpreter/interpreter.h"
 
 #include <fstream>
 #include <memory>
 
 #include "src/ast/prettyprinter.h"
 #include "src/builtins/builtins-arguments.h"
 #include "src/builtins/builtins-constructor.h"
 #include "src/code-factory.h"
 #include "src/compilation-info.h"
 #include "src/compiler.h"
 #include "src/factory.h"
+#include "src/ic/accessor-assembler.h"
 #include "src/interpreter/bytecode-flags.h"
 #include "src/interpreter/bytecode-generator.h"
 #include "src/interpreter/bytecodes.h"
 #include "src/interpreter/interpreter-assembler.h"
 #include "src/interpreter/interpreter-intrinsics.h"
 #include "src/log.h"
 #include "src/zone/zone.h"
 
 namespace v8 {
 namespace internal {
@@ skipping 424 matching lines @@
 //
 // Stores the value of register <src> to register <dst>.
 void Interpreter::DoMov(InterpreterAssembler* assembler) {
   Node* src_index = __ BytecodeOperandReg(0);
   Node* src_value = __ LoadRegister(src_index);
   Node* dst_index = __ BytecodeOperandReg(1);
   __ StoreRegister(src_value, dst_index);
   __ Dispatch();
 }
 
-Node* Interpreter::BuildLoadGlobal(Callable ic, Node* context, Node* name_index,
-                                   Node* feedback_slot,
-                                   InterpreterAssembler* assembler) {
+void Interpreter::BuildLoadGlobal(
+    const std::function<compiler::Node*()>& gen_name_index, Node* feedback_slot,

[Igor Sheludko, 2017/02/08 15:42:20]

Given that you already sawed LoadGlobalIC into building blocks I think you can
just pass the name_operand_index and slot_operand_index to this function and
load them here. 

Ross, WDYT?

[rmcilroy, 2017/02/08 17:39:14]

+1 I think you should be able to just pass the name_index as a node directly,
no?

[Igor Sheludko, 2017/02/08 17:44:40]

The idea is to remove everything related to name from the fast path where we
load a property cell from the vector and check if it contains a hole.

[Igor Sheludko, 2017/02/08 17:46:25]

And in my comment above I meant to pass indices as ints.

[rmcilroy, 2017/02/08 18:38:21]

I see. I could live with the bytecode operand index being passed as an argument.

[jgruber, 2017/02/09 09:32:51]

This is what I went with, BuildLoadGlobal now takes an integer name_index_index
and slot_index and loads the parameters as needed.

+    TypeofMode typeof_mode, InterpreterAssembler* assembler) {
   // Load the global via the LoadGlobalIC.
-  Node* code_target = __ HeapConstant(ic.code());
-  Node* name = __ LoadConstantPoolEntry(name_index);
   Node* smi_slot = __ SmiTag(feedback_slot);
   Node* feedback_vector = __ LoadFeedbackVector();
-  return __ CallStub(ic.descriptor(), code_target, context, name, smi_slot,
-                     feedback_vector);
+
+  AccessorAssembler accessor_asm(assembler->state());
+
+  Label try_handler(assembler, Label::kDeferred),
+      miss(assembler, Label::kDeferred);
+
+  // Fast path without frame construction for the data case.
+  {
+    Label out(assembler);

[rmcilroy, 2017/02/08 18:38:21]

nit - "done" is used more commonly. 

[jgruber, 2017/02/09 09:32:51]

Done.

+    Variable var_result(assembler, MachineRepresentation::kTagged);
+    ExitPoint exit_point(assembler, &out, &var_result);
+
+    AccessorAssembler::LoadICParameters p(nullptr, nullptr, nullptr, smi_slot,

[rmcilroy, 2017/02/08 18:38:21]

nit - params

[jgruber, 2017/02/09 09:32:51]

These have been removed.

+                                          feedback_vector);
+    accessor_asm.LoadGlobalICData(&p, &exit_point, &try_handler, &miss);

[Igor Sheludko, 2017/02/08 15:42:20]

s/&p/feedback_vector, smi_slot/

[jgruber, 2017/02/09 09:32:51]

Done. Also removed smi tagging for the fast path.

+
+    __ Bind(&out);
+    __ SetAccumulator(var_result.value());
+    __ Dispatch();
+  }
+
+  // Slow path with frame construction.
+  {
+    Label out(assembler);

[rmcilroy, 2017/02/08 18:38:21]

ditto.

[jgruber, 2017/02/09 09:32:51]

Done.

+    Variable var_result(assembler, MachineRepresentation::kTagged);
+    ExitPoint exit_point(assembler, &out, &var_result);
+
+    __ Bind(&try_handler);
+    {
+      Node* context = __ GetContext();
+      Node* name = __ LoadConstantPoolEntry(gen_name_index());
+
+      AccessorAssembler::LoadICParameters p(context, nullptr, name, smi_slot,

[rmcilroy, 2017/02/08 18:38:21]

ditto

[jgruber, 2017/02/09 09:32:51]

Done.

+                                            feedback_vector);
+      accessor_asm.LoadGlobalICHandler(&p, typeof_mode, &exit_point, &miss);
+    }
+
+    __ Bind(&miss);
+    {
+      Node* context = __ GetContext();
+      Node* name = __ LoadConstantPoolEntry(gen_name_index());
+
+      AccessorAssembler::LoadICParameters p(context, nullptr, name, smi_slot,
+                                            feedback_vector);
+      accessor_asm.LoadGlobalICMiss(&p, &exit_point);
+    }
+
+    __ Bind(&out);
+    {
+      __ SetAccumulator(var_result.value());
+      __ Dispatch();
+    }
+  }
 }
 
 // LdaGlobal <name_index> <slot>
 //
 // Load the global with name in constant pool entry <name_index> into the
 // accumulator using FeedBackVector slot <slot> outside of a typeof.
 void Interpreter::DoLdaGlobal(InterpreterAssembler* assembler) {
-  Callable ic =
-      CodeFactory::LoadGlobalICInOptimizedCode(isolate_, NOT_INSIDE_TYPEOF);
+  Node* raw_slot = __ BytecodeOperandIdx(1);
 
-  Node* context = __ GetContext();
+  auto gen_name_index = [=]() { return __ BytecodeOperandIdx(0); };
 
-  Node* name_index = __ BytecodeOperandIdx(0);
-  Node* raw_slot = __ BytecodeOperandIdx(1);
-  Node* result = BuildLoadGlobal(ic, context, name_index, raw_slot, assembler);
-  __ SetAccumulator(result);
-  __ Dispatch();
+  BuildLoadGlobal(gen_name_index, raw_slot, NOT_INSIDE_TYPEOF, assembler);
 }
 
 // LdaGlobalInsideTypeof <name_index> <slot>
 //
 // Load the global with name in constant pool entry <name_index> into the
 // accumulator using FeedBackVector slot <slot> inside of a typeof.
 void Interpreter::DoLdaGlobalInsideTypeof(InterpreterAssembler* assembler) {
-  Callable ic =
-      CodeFactory::LoadGlobalICInOptimizedCode(isolate_, INSIDE_TYPEOF);
+  Node* raw_slot = __ BytecodeOperandIdx(1);
 
-  Node* context = __ GetContext();
+  auto gen_name_index = [=]() { return __ BytecodeOperandIdx(0); };
 
-  Node* name_index = __ BytecodeOperandIdx(0);
-  Node* raw_slot = __ BytecodeOperandIdx(1);
-  Node* result = BuildLoadGlobal(ic, context, name_index, raw_slot, assembler);
-  __ SetAccumulator(result);
-  __ Dispatch();
+  BuildLoadGlobal(gen_name_index, raw_slot, INSIDE_TYPEOF, assembler);
 }
 
 void Interpreter::DoStaGlobal(Callable ic, InterpreterAssembler* assembler) {
   // Get the global object.
   Node* context = __ GetContext();
   Node* native_context = __ LoadNativeContext(context);
   Node* global =
       __ LoadContextElement(native_context, Context::EXTENSION_INDEX);
 
   // Store the global via the StoreIC.
@@ skipping 167 matching lines @@
 // Lookup the object with the name in constant pool entry |name_index|
 // dynamically without causing a NoReferenceError.
 void Interpreter::DoLdaLookupContextSlotInsideTypeof(
     InterpreterAssembler* assembler) {
   DoLdaLookupContextSlot(Runtime::kLoadLookupSlotInsideTypeof, assembler);
 }
 
 void Interpreter::DoLdaLookupGlobalSlot(Runtime::FunctionId function_id,
                                         InterpreterAssembler* assembler) {
   Node* context = __ GetContext();
-  Node* name_index = __ BytecodeOperandIdx(0);
   Node* feedback_slot = __ BytecodeOperandIdx(1);
   Node* depth = __ BytecodeOperandUImm(2);
 
   Label slowpath(assembler, Label::kDeferred);
 
   // Check for context extensions to allow the fast path
   __ GotoIfHasContextExtensionUpToDepth(context, depth, &slowpath);
 
   // Fast path does a normal load global
   {
-    Callable ic = CodeFactory::LoadGlobalICInOptimizedCode(
-        isolate_, function_id == Runtime::kLoadLookupSlotInsideTypeof
-                      ? INSIDE_TYPEOF
-                      : NOT_INSIDE_TYPEOF);
-    Node* result =
-        BuildLoadGlobal(ic, context, name_index, feedback_slot, assembler);
-    __ SetAccumulator(result);
-    __ Dispatch();
+    auto gen_name_index = [=]() { return __ BytecodeOperandIdx(0); };
+
+    TypeofMode typeof_mode = function_id == Runtime::kLoadLookupSlotInsideTypeof
+                                 ? INSIDE_TYPEOF
+                                 : NOT_INSIDE_TYPEOF;
+
+    BuildLoadGlobal(gen_name_index, feedback_slot, typeof_mode, assembler);
   }
 
   // Slow path when we have to call out to the runtime
   __ Bind(&slowpath);
   {
+    Node* name_index = __ BytecodeOperandIdx(0);
     Node* name = __ LoadConstantPoolEntry(name_index);
     Node* result = __ CallRuntime(function_id, context, name);
     __ SetAccumulator(result);
     __ Dispatch();
   }
 }
 
 // LdaLookupGlobalSlot <name_index> <feedback_slot> <depth>
 //
 // Lookup the object with the name in constant pool entry |name_index|
@@ skipping 2599 matching lines @@
   __ StoreObjectField(generator, JSGeneratorObject::kContinuationOffset,
       __ SmiTag(new_state));
   __ SetAccumulator(old_state);
 
   __ Dispatch();
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8