[Interpreter] Move context chain search loop to handler

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"
@@ skipping 497 matching lines @@
 
 // StaGlobalStrict <name_index> <slot>
 //
 // Store the value in the accumulator into the global with name in constant pool
 // entry <name_index> using FeedBackVector slot <slot> in strict mode.
 void Interpreter::DoStaGlobalStrict(InterpreterAssembler* assembler) {
   Callable ic = CodeFactory::StoreICInOptimizedCode(isolate_, STRICT);
   DoStaGlobal(ic, assembler);
 }
 
+compiler::Node* Interpreter::GetContextAtDepth(InterpreterAssembler* assembler,
+                                               Node* root_context,
+                                               Node* depth) {
+  Variable context(assembler, MachineRepresentation::kTaggedPointer);
+  context.Bind(root_context);
+
+  Label context_found(assembler);
+
+  // Fast path if the depth is 0

[rmcilroy, 2016/09/12 17:07:15]

Nit - full stop on comments.

[Leszek Swirski, 2016/09/13 09:42:34]

Done.

+  __ GotoIf(__ WordEqual(depth, __ Int32Constant(0)), &context_found);
+
+  // Loop until the depth is 0
+  Variable cur_depth(assembler, MachineRepresentation::kWord32);
+
+  Variable* context_search_loop_variables[2] = {&cur_depth, &context};
+  Label context_search(assembler, 2, context_search_loop_variables);
+
+  cur_depth.Bind(depth);
+  __ Goto(&context_search);
+  __ Bind(&context_search);
+  {
+    cur_depth.Bind(__ Int32Sub(cur_depth.value(), __ Int32Constant(1)));
+    context.Bind(__ LoadContextSlot(context.value(), Context::PREVIOUS_INDEX));

[rmcilroy, 2016/09/12 17:07:15]

Nit - swapping these around might give slightly better codegen (merge subtract
instructions modification of flags with branch check). You can see generated
code with --trace-ignition-codegen or some similarly named flag)

[Leszek Swirski, 2016/09/13 09:42:34]

Sadly there's no improvement in the codegen, but agree that there could be, so
I'll leave this reordered in case that optimisation comes along later.

+
+    __ Branch(__ WordEqual(cur_depth.value(), __ Int32Constant(0)),

[rmcilroy, 2016/09/12 17:07:15]

You can do BranchIfWordEqual(...)

[Leszek Swirski, 2016/09/13 09:42:34]

Done.

+              &context_found, &context_search);
+  }
+
+  __ Bind(&context_found);
+  return context.value();
+}
+
 compiler::Node* Interpreter::BuildLoadContextSlot(
     InterpreterAssembler* assembler) {
   Node* reg_index = __ BytecodeOperandReg(0);
-  Node* context = __ LoadRegister(reg_index);
+  Node* root_context = __ LoadRegister(reg_index);
   Node* slot_index = __ BytecodeOperandIdx(1);
-  return __ LoadContextSlot(context, slot_index);
+  Node* depth = __ BytecodeOperandIdx(2);
+
+  return __ LoadContextSlot(GetContextAtDepth(assembler, root_context, depth),

[rmcilroy, 2016/09/12 17:07:15]

Nit - do the GetContextAtDepth on a different line to make the schedule of the
code obvious.

[Leszek Swirski, 2016/09/13 09:42:34]

Done.

+                            slot_index);
 }
 
 // LdaContextSlot <context> <slot_index>

[rmcilroy, 2016/09/12 17:07:15]

Please update these comments (with <depth> and /s/context/root_context/

[Leszek Swirski, 2016/09/13 09:42:34]

Done.

 //
 // Load the object in |slot_index| of |context| into the accumulator.
 void Interpreter::DoLdaContextSlot(InterpreterAssembler* assembler) {
   Node* result = BuildLoadContextSlot(assembler);
   __ SetAccumulator(result);
   __ Dispatch();
 }
 
 // LdrContextSlot <context> <slot_index> <reg>
 //
 // Load the object in <slot_index> of <context> into register <reg>.
 void Interpreter::DoLdrContextSlot(InterpreterAssembler* assembler) {
   Node* result = BuildLoadContextSlot(assembler);
-  Node* destination = __ BytecodeOperandReg(2);
+  Node* destination = __ BytecodeOperandReg(3);
   __ StoreRegister(result, destination);
   __ Dispatch();
 }
 
 // StaContextSlot <context> <slot_index>
 //
 // Stores the object in the accumulator into |slot_index| of |context|.
 void Interpreter::DoStaContextSlot(InterpreterAssembler* assembler) {
   Node* value = __ GetAccumulator();
   Node* reg_index = __ BytecodeOperandReg(0);
-  Node* context = __ LoadRegister(reg_index);
+  Node* root_context = __ LoadRegister(reg_index);
   Node* slot_index = __ BytecodeOperandIdx(1);
-  __ StoreContextSlot(context, slot_index, value);
+  Node* depth = __ BytecodeOperandIdx(2);
+
+  __ StoreContextSlot(GetContextAtDepth(assembler, root_context, depth),

[rmcilroy, 2016/09/12 17:07:15]

Ditto

[Leszek Swirski, 2016/09/13 09:42:34]

Done.

+                      slot_index, value);
   __ Dispatch();
 }
 
 void Interpreter::DoLdaLookupSlot(Runtime::FunctionId function_id,
                                   InterpreterAssembler* assembler) {
   Node* index = __ BytecodeOperandIdx(0);
   Node* name = __ LoadConstantPoolEntry(index);
   Node* context = __ GetContext();
   Node* result = __ CallRuntime(function_id, context, name);
   __ SetAccumulator(result);
@@ skipping 1892 matching lines @@
   __ StoreObjectField(generator, JSGeneratorObject::kContinuationOffset,
       __ SmiTag(new_state));
   __ SetAccumulator(old_state);
 
   __ Dispatch();
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8