[Interpreter] Move context chain search loop to handler

src/interpreter/interpreter.cc

Index: src/interpreter/interpreter.cc
diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc
index 7a38a63e128e025b18a30cc6e6a1cce18835b479..906ad83217839447387e4dd1074a6496ed0ec014 100644
--- a/src/interpreter/interpreter.cc
+++ b/src/interpreter/interpreter.cc
@@ -515,12 +515,47 @@ void Interpreter::DoStaGlobalStrict(InterpreterAssembler* assembler) {
   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.

@@ -537,7 +572,7 @@ void Interpreter::DoLdaContextSlot(InterpreterAssembler* assembler) {
 // 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();
 }
@@ -548,9 +583,12 @@ void Interpreter::DoLdrContextSlot(InterpreterAssembler* assembler) {
 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();
 }