[Interpreter] Avoid binding bytecode_array to a variable in CallEpilogue

src/interpreter/interpreter-assembler.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-assembler.h"
 
 #include <limits>
 #include <ostream>
 
 #include "src/code-factory.h"
@@ skipping 15 matching lines @@
                                            Bytecode bytecode,
                                            OperandScale operand_scale)
     : compiler::CodeStubAssembler(isolate, zone,
                                   InterpreterDispatchDescriptor(isolate),
                                   Code::ComputeFlags(Code::BYTECODE_HANDLER),
                                   Bytecodes::ToString(bytecode), 0),
       bytecode_(bytecode),
       operand_scale_(operand_scale),
       accumulator_(this, MachineRepresentation::kTagged),
       accumulator_use_(AccumulatorUse::kNone),
-      bytecode_array_(this, MachineRepresentation::kTagged),
+      made_call_(false),
       disable_stack_check_across_call_(false),
       stack_pointer_before_call_(nullptr) {
   accumulator_.Bind(
       Parameter(InterpreterDispatchDescriptor::kAccumulatorParameter));
-  bytecode_array_.Bind(
-      Parameter(InterpreterDispatchDescriptor::kBytecodeArrayParameter));
   if (FLAG_trace_ignition) {
     TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry);
   }
 }
 
 InterpreterAssembler::~InterpreterAssembler() {
   // If the following check fails the handler does not use the
   // accumulator in the way described in the bytecode definitions in
   // bytecodes.h.
   DCHECK_EQ(accumulator_use_, Bytecodes::GetAccumulatorUse(bytecode_));
@@ skipping 25 matching lines @@
 
 Node* InterpreterAssembler::BytecodeOffset() {
   return Parameter(InterpreterDispatchDescriptor::kBytecodeOffsetParameter);
 }
 
 Node* InterpreterAssembler::RegisterFileRawPointer() {
   return Parameter(InterpreterDispatchDescriptor::kRegisterFileParameter);
 }
 
 Node* InterpreterAssembler::BytecodeArrayTaggedPointer() {
-  return bytecode_array_.value();
+  if (made_call_) {
+    // If we have made a call, restore bytecode array from stack frame in case
+    // the debugger has swapped us to the patched debugger bytecode array.
+    return LoadRegister(

[epertoso, 2016/04/18 14:23:27]

You probably want to reset made_call_ to false here.

[rmcilroy, 2016/04/18 14:59:15]

No I don't think so, if we did that then if there was a branch after the call
where both sides of the branch do a Dispatch (e.g., the Jump bytecodes), then
setting this to false here would make one side of the branch get the reloaded
value, while the other would get the value in the parameter - both should get
the reloaded version if the handler made a call.

As mentioned offline, we get a new interpreter assembler for every bytecode
handler, so this should be ok.

+        InterpreterFrameConstants::kBytecodeArrayFromRegisterPointer);
+  } else {
+    return Parameter(InterpreterDispatchDescriptor::kBytecodeArrayParameter);
+  }
 }
 
 Node* InterpreterAssembler::DispatchTableRawPointer() {
   return Parameter(InterpreterDispatchDescriptor::kDispatchTableParameter);
 }
 
 Node* InterpreterAssembler::RegisterLocation(Node* reg_index) {
   return IntPtrAdd(RegisterFileRawPointer(), RegisterFrameOffset(reg_index));
 }
 
@@ skipping 318 matching lines @@
 }
 
 void InterpreterAssembler::CallPrologue() {
   StoreRegister(SmiTag(BytecodeOffset()),
                 InterpreterFrameConstants::kBytecodeOffsetFromRegisterPointer);
 
   if (FLAG_debug_code && !disable_stack_check_across_call_) {
     DCHECK(stack_pointer_before_call_ == nullptr);
     stack_pointer_before_call_ = LoadStackPointer();
   }
+  made_call_ = true;
 }
 
 void InterpreterAssembler::CallEpilogue() {
   if (FLAG_debug_code && !disable_stack_check_across_call_) {
     Node* stack_pointer_after_call = LoadStackPointer();
     Node* stack_pointer_before_call = stack_pointer_before_call_;
     stack_pointer_before_call_ = nullptr;
     AbortIfWordNotEqual(stack_pointer_before_call, stack_pointer_after_call,
                         kUnexpectedStackPointer);
   }
-
-  // Restore bytecode array from stack frame in case the debugger has swapped us
-  // to the patched debugger bytecode array.
-  bytecode_array_.Bind(LoadRegister(
-      InterpreterFrameConstants::kBytecodeArrayFromRegisterPointer));
 }
 
 Node* InterpreterAssembler::CallJS(Node* function, Node* context,
                                    Node* first_arg, Node* arg_count,
                                    TailCallMode tail_call_mode) {
   Callable callable =
       CodeFactory::InterpreterPushArgsAndCall(isolate(), tail_call_mode);
   Node* code_target = HeapConstant(callable.code());
   return CallStub(callable.descriptor(), code_target, context, arg_count,
                   first_arg, function);
@@ skipping 274 matching lines @@
     V8_TARGET_ARCH_S390
   return true;
 #else
 #error "Unknown Architecture"
 #endif
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8