Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(61)

Unified Diff: src/compiler/bytecode-graph-builder.cc

Issue 1514413002: [Interpreter] Generate valid FrameStates in the Bytecode Graph Builder. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@int_materialize_sf
Patch Set: Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: src/compiler/bytecode-graph-builder.cc
diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc
index 96c1415c5fbf5168b00d0b76ca61b1ab65a0c80f..7c0481d842e270063226246831ba9330371171b5 100644
--- a/src/compiler/bytecode-graph-builder.cc
+++ b/src/compiler/bytecode-graph-builder.cc
@@ -14,7 +14,6 @@ namespace internal {
namespace compiler {
// Issues:
-// - Need to deal with FrameState / FrameStateBeforeAndAfter / StateValue.
// - Scopes - intimately tied to AST. Need to eval what is needed.
// - Need to resolve closure parameter treatment.
BytecodeGraphBuilder::Environment::Environment(BytecodeGraphBuilder* builder,
@@ -28,10 +27,13 @@ BytecodeGraphBuilder::Environment::Environment(BytecodeGraphBuilder* builder,
context_(context),
control_dependency_(control_dependency),
effect_dependency_(control_dependency),
- values_(builder->local_zone()) {
+ values_(builder->local_zone()),
+ parameters_state_values_(nullptr),
+ registers_state_values_(nullptr),
+ accumulator_state_values_(nullptr) {
// The layout of values_ is:
//
- // [receiver] [parameters] [registers]
+ // [receiver] [parameters] [registers] [accumulator]
//
// parameter[0] is the receiver (this), parameters 1..N are the
// parameters supplied to the method (arg0..argN-1). The accumulator
@@ -51,7 +53,8 @@ BytecodeGraphBuilder::Environment::Environment(BytecodeGraphBuilder* builder,
values()->insert(values()->end(), register_count, undefined_constant);
// Accumulator
- accumulator_ = undefined_constant;
+ accumulator_base_ = static_cast<int>(values()->size());
+ values()->push_back(undefined_constant);
}
@@ -88,12 +91,12 @@ Node* BytecodeGraphBuilder::Environment::LookupRegister(
void BytecodeGraphBuilder::Environment::BindAccumulator(Node* node) {
- accumulator_ = node;
+ values()->at(accumulator_base_) = node;
}
Node* BytecodeGraphBuilder::Environment::LookupAccumulator() const {
- return accumulator_;
+ return values()->at(accumulator_base_);
}
@@ -107,17 +110,113 @@ void BytecodeGraphBuilder::Environment::MarkAsUnreachable() {
}
+void BytecodeGraphBuilder::Environment::UpdateStateValues(Node** state_values,
+ int offset,
+ int count) {
+ bool should_update = false;
+ Node** env_values = (count == 0) ? nullptr : &values()->at(offset);
+ if (*state_values == NULL) {
+ should_update = true;
+ } else {
+ DCHECK_EQ((*state_values)->InputCount(), count);
+ DCHECK_LE(static_cast<size_t>(offset + count), values()->size());
+ for (int i = 0; i < count; i++) {
+ if ((*state_values)->InputAt(i) != env_values[i]) {
+ should_update = true;
+ break;
+ }
+ }
+ }
+ if (should_update) {
+ const Operator* op = common()->StateValues(count);
+ (*state_values) = graph()->NewNode(op, count, env_values);
+ }
+}
+
+
+Node* BytecodeGraphBuilder::Environment::Checkpoint(
+ BailoutId bailout_id, AccumulatorUpdateMode update_mode) {
+ if (!builder()->info()->is_deoptimization_enabled()) {
+ return builder()->jsgraph()->EmptyFrameState();
+ }
+
+ // TODO(rmcilroy): Consider using StateValuesCache for some state values.
+ UpdateStateValues(&parameters_state_values_, 0, parameter_count());
+ UpdateStateValues(&registers_state_values_, register_base(),
+ register_count());
+ UpdateStateValues(&accumulator_state_values_, accumulator_base(), 1);
+
+ OutputFrameStateCombine combine =
+ update_mode == AccumulatorUpdateMode::kOutputIgnored
Jarin 2015/12/13 20:26:30 I do not understand why you differentiate between
rmcilroy 2015/12/16 15:40:56 We need this because some bytecodes don't modify t
Jarin 2015/12/17 09:44:05 Acknowledged.
+ ? OutputFrameStateCombine::Ignore()
+ : OutputFrameStateCombine::Push();
+ const Operator* op = common()->FrameState(
+ bailout_id, combine, builder()->frame_state_function_info());
+
+ Node* result = graph()->NewNode(
+ op, parameters_state_values_, registers_state_values_,
+ accumulator_state_values_, Context(), builder()->GetFunctionClosure(),
+ builder()->graph()->start());
+
+ return result;
+}
+
+
+// Helper for generating frame states for before and after a bytecode.
+class BytecodeGraphBuilder::FrameStateBeforeAndAfter {
+ public:
+ FrameStateBeforeAndAfter(BytecodeGraphBuilder* builder,
+ const interpreter::BytecodeArrayIterator& iterator)
+ : builder_(builder), id_after_(BailoutId::None()) {
+ BailoutId id_before(iterator.current_offset());
+ frame_state_before_ = builder_->environment()->Checkpoint(
+ id_before, AccumulatorUpdateMode::kOutputIgnored);
+ id_after_ = BailoutId(id_before.ToInt() + iterator.current_bytecode_size());
+ }
+
+ void AddToNode(Node* node, AccumulatorUpdateMode update_mode) {
+ int count = OperatorProperties::GetFrameStateInputCount(node->op());
+ DCHECK_LE(count, 2);
+
+ if (count >= 1) {
+ // Add the frame state for after the operation.
+ DCHECK_EQ(IrOpcode::kDead,
+ NodeProperties::GetFrameStateInput(node, 0)->opcode());
+ Node* frame_state_after =
+ builder_->environment()->Checkpoint(id_after_, update_mode);
+ NodeProperties::ReplaceFrameStateInput(node, 0, frame_state_after);
+ }
+
+ if (count >= 2) {
+ // Add the frame state for before the operation.
+ DCHECK_EQ(IrOpcode::kDead,
+ NodeProperties::GetFrameStateInput(node, 1)->opcode());
+ NodeProperties::ReplaceFrameStateInput(node, 1, frame_state_before_);
+ }
+ }
+
+ private:
+ BytecodeGraphBuilder* builder_;
+ Node* frame_state_before_;
+ BailoutId id_after_;
+};
+
+
BytecodeGraphBuilder::BytecodeGraphBuilder(Zone* local_zone,
CompilationInfo* compilation_info,
JSGraph* jsgraph)
: local_zone_(local_zone),
info_(compilation_info),
jsgraph_(jsgraph),
+ bytecode_array_(handle(info()->shared_info()->bytecode_array())),
+ frame_state_function_info_(common()->CreateFrameStateFunctionInfo(
+ FrameStateType::kInterpretedFunction,
+ bytecode_array()->parameter_count(),
+ bytecode_array()->register_count(), info()->shared_info(),
+ CALL_MAINTAINS_NATIVE_CONTEXT)),
input_buffer_size_(0),
input_buffer_(nullptr),
- exit_controls_(local_zone) {
- bytecode_array_ = handle(info()->shared_info()->bytecode_array());
-}
+ exit_controls_(local_zone) {}
Node* BytecodeGraphBuilder::GetNewTarget() {
@@ -198,18 +297,6 @@ VectorSlotPair BytecodeGraphBuilder::CreateVectorSlotPair(int slot_id) {
}
-// TODO(mythria): Replace this function with one which adds real frame state.
-// Also add before and after frame states and checkpointing if required.
-void BytecodeGraphBuilder::AddEmptyFrameStateInputs(Node* node) {
- int frame_state_count =
- OperatorProperties::GetFrameStateInputCount(node->op());
- for (int i = 0; i < frame_state_count; i++) {
- NodeProperties::ReplaceFrameStateInput(node, i,
- jsgraph()->EmptyFrameState());
- }
-}
-
-
bool BytecodeGraphBuilder::CreateGraph(bool stack_check) {
// Set up the basic structure of the graph. Outputs for {Start} are
// the formal parameters (including the receiver) plus context and
@@ -349,13 +436,14 @@ void BytecodeGraphBuilder::VisitMov(
void BytecodeGraphBuilder::BuildLoadGlobal(
const interpreter::BytecodeArrayIterator& iterator,
TypeofMode typeof_mode) {
+ FrameStateBeforeAndAfter states(this, iterator);
Handle<Name> name =
Handle<Name>::cast(iterator.GetConstantForIndexOperand(0));
VectorSlotPair feedback = CreateVectorSlotPair(iterator.GetIndexOperand(1));
const Operator* op = javascript()->LoadGlobal(name, feedback, typeof_mode);
Node* node = NewNode(op, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -418,6 +506,7 @@ void BytecodeGraphBuilder::VisitLdaGlobalInsideTypeofStrictWide(
void BytecodeGraphBuilder::BuildStoreGlobal(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Jarin 2015/12/13 20:26:30 Could we please implement the minimal machinery ne
rmcilroy 2015/12/16 15:40:56 As discussed offline, keeping this machinary in si
Jarin 2015/12/17 09:44:05 Ack. Even though I am wondering whether the FrameS
rmcilroy 2015/12/17 14:43:48 Good point on the fact that the OutputIgnore/Outpu
Handle<Name> name =
Handle<Name>::cast(iterator.GetConstantForIndexOperand(0));
VectorSlotPair feedback = CreateVectorSlotPair(iterator.GetIndexOperand(1));
@@ -426,7 +515,7 @@ void BytecodeGraphBuilder::BuildStoreGlobal(
const Operator* op =
javascript()->StoreGlobal(language_mode(), name, feedback);
Node* node = NewNode(op, value, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputIgnored);
}
@@ -482,14 +571,13 @@ void BytecodeGraphBuilder::VisitStaContextSlot(
javascript()->StoreContext(0, iterator.GetIndexOperand(1));
Node* context = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Node* value = environment()->LookupAccumulator();
- Node* node = NewNode(op, context, value);
- CHECK(node != nullptr);
- environment()->BindAccumulator(value);
+ NewNode(op, context, value);
}
void BytecodeGraphBuilder::BuildNamedLoad(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Handle<Name> name =
Handle<Name>::cast(iterator.GetConstantForIndexOperand(1));
@@ -497,7 +585,7 @@ void BytecodeGraphBuilder::BuildNamedLoad(
const Operator* op = javascript()->LoadNamed(language_mode(), name, feedback);
Node* node = NewNode(op, object, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -532,13 +620,14 @@ void BytecodeGraphBuilder::VisitLoadICStrictWide(
void BytecodeGraphBuilder::BuildKeyedLoad(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* key = environment()->LookupAccumulator();
Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
VectorSlotPair feedback = CreateVectorSlotPair(iterator.GetIndexOperand(1));
const Operator* op = javascript()->LoadProperty(language_mode(), feedback);
Node* node = NewNode(op, object, key, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -573,6 +662,7 @@ void BytecodeGraphBuilder::VisitKeyedLoadICStrictWide(
void BytecodeGraphBuilder::BuildNamedStore(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* value = environment()->LookupAccumulator();
Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Handle<Name> name =
@@ -582,8 +672,7 @@ void BytecodeGraphBuilder::BuildNamedStore(
const Operator* op =
javascript()->StoreNamed(language_mode(), name, feedback);
Node* node = NewNode(op, object, value, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
- environment()->BindAccumulator(value);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputIgnored);
}
@@ -617,6 +706,7 @@ void BytecodeGraphBuilder::VisitStoreICStrictWide(
void BytecodeGraphBuilder::BuildKeyedStore(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* value = environment()->LookupAccumulator();
Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Node* key = environment()->LookupRegister(iterator.GetRegisterOperand(1));
@@ -624,8 +714,7 @@ void BytecodeGraphBuilder::BuildKeyedStore(
const Operator* op = javascript()->StoreProperty(language_mode(), feedback);
Node* node = NewNode(op, object, key, value, BuildLoadFeedbackVector());
- AddEmptyFrameStateInputs(node);
- environment()->BindAccumulator(value);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputIgnored);
}
@@ -680,7 +769,6 @@ void BytecodeGraphBuilder::VisitCreateClosure(
iterator.GetImmediateOperand(1) ? TENURED : NOT_TENURED;
const Operator* op = javascript()->CreateClosure(shared_info, tenured);
Node* closure = NewNode(op);
- AddEmptyFrameStateInputs(closure);
environment()->BindAccumulator(closure);
}
@@ -703,9 +791,11 @@ void BytecodeGraphBuilder::VisitCreateUnmappedArguments(
}
-void BytecodeGraphBuilder::BuildCreateLiteral(const Operator* op) {
+void BytecodeGraphBuilder::BuildCreateLiteral(
+ const Operator* op, const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* literal = NewNode(op, GetFunctionClosure());
- AddEmptyFrameStateInputs(literal);
+ states.AddToNode(literal, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(literal);
}
@@ -718,7 +808,7 @@ void BytecodeGraphBuilder::BuildCreateRegExpLiteral(
int literal_flags = iterator.GetImmediateOperand(2);
const Operator* op = javascript()->CreateLiteralRegExp(
constant_pattern, literal_flags, literal_index);
- BuildCreateLiteral(op);
+ BuildCreateLiteral(op, iterator);
}
@@ -742,7 +832,7 @@ void BytecodeGraphBuilder::BuildCreateArrayLiteral(
int literal_flags = iterator.GetImmediateOperand(2);
const Operator* op = javascript()->CreateLiteralArray(
constant_elements, literal_flags, literal_index);
- BuildCreateLiteral(op);
+ BuildCreateLiteral(op, iterator);
}
@@ -766,7 +856,7 @@ void BytecodeGraphBuilder::BuildCreateObjectLiteral(
int literal_flags = iterator.GetImmediateOperand(2);
const Operator* op = javascript()->CreateLiteralObject(
constant_properties, literal_flags, literal_index);
- BuildCreateLiteral(op);
+ BuildCreateLiteral(op, iterator);
}
@@ -801,6 +891,7 @@ Node* BytecodeGraphBuilder::ProcessCallArguments(const Operator* call_op,
void BytecodeGraphBuilder::BuildCall(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
// TODO(rmcilroy): Set receiver_hint correctly based on whether the receiver
// register has been loaded with null / undefined explicitly or we are sure it
// is not null / undefined.
@@ -813,7 +904,7 @@ void BytecodeGraphBuilder::BuildCall(
const Operator* call = javascript()->CallFunction(
arg_count + 2, language_mode(), feedback, receiver_hint);
Node* value = ProcessCallArguments(call, callee, receiver, arg_count + 2);
- AddEmptyFrameStateInputs(value);
+ states.AddToNode(value, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(value);
}
@@ -832,6 +923,7 @@ void BytecodeGraphBuilder::VisitCallWide(
void BytecodeGraphBuilder::VisitCallJSRuntime(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* callee = BuildLoadNativeContextField(iterator.GetIndexOperand(0));
interpreter::Register receiver = iterator.GetRegisterOperand(1);
size_t arg_count = iterator.GetCountOperand(2);
@@ -840,7 +932,7 @@ void BytecodeGraphBuilder::VisitCallJSRuntime(
const Operator* call =
javascript()->CallFunction(arg_count + 2, language_mode());
Node* value = ProcessCallArguments(call, callee, receiver, arg_count + 2);
- AddEmptyFrameStateInputs(value);
+ states.AddToNode(value, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(value);
}
@@ -861,6 +953,7 @@ Node* BytecodeGraphBuilder::ProcessCallRuntimeArguments(
void BytecodeGraphBuilder::VisitCallRuntime(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Runtime::FunctionId functionId =
static_cast<Runtime::FunctionId>(iterator.GetIndexOperand(0));
interpreter::Register first_arg = iterator.GetRegisterOperand(1);
@@ -869,7 +962,7 @@ void BytecodeGraphBuilder::VisitCallRuntime(
// Create node to perform the runtime call.
const Operator* call = javascript()->CallRuntime(functionId, arg_count);
Node* value = ProcessCallRuntimeArguments(call, first_arg, arg_count);
- AddEmptyFrameStateInputs(value);
+ states.AddToNode(value, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(value);
}
@@ -893,6 +986,7 @@ Node* BytecodeGraphBuilder::ProcessCallNewArguments(
void BytecodeGraphBuilder::VisitNew(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
interpreter::Register callee = iterator.GetRegisterOperand(0);
interpreter::Register first_arg = iterator.GetRegisterOperand(1);
size_t arg_count = iterator.GetCountOperand(2);
@@ -901,30 +995,31 @@ void BytecodeGraphBuilder::VisitNew(
const Operator* call = javascript()->CallConstruct(
static_cast<int>(arg_count) + 2, VectorSlotPair());
Node* value = ProcessCallNewArguments(call, callee, first_arg, arg_count + 2);
- AddEmptyFrameStateInputs(value);
+ states.AddToNode(value, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(value);
}
void BytecodeGraphBuilder::VisitThrow(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* value = environment()->LookupAccumulator();
// TODO(mythria): Change to Runtime::kThrow when we have deoptimization
// information support in the interpreter.
NewNode(javascript()->CallRuntime(Runtime::kReThrow, 1), value);
Node* control = NewNode(common()->Throw(), value);
+ states.AddToNode(control, AccumulatorUpdateMode::kOutputIgnored);
UpdateControlDependencyToLeaveFunction(control);
- environment()->BindAccumulator(value);
}
void BytecodeGraphBuilder::BuildBinaryOp(
const Operator* js_op, const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* left = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Node* right = environment()->LookupAccumulator();
Node* node = NewNode(js_op, left, right);
-
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -1009,24 +1104,24 @@ void BytecodeGraphBuilder::VisitShiftRightLogical(
void BytecodeGraphBuilder::VisitInc(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
const Operator* js_op =
javascript()->Add(language_mode(), BinaryOperationHints::Any());
Node* node = NewNode(js_op, environment()->LookupAccumulator(),
jsgraph()->OneConstant());
-
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
void BytecodeGraphBuilder::VisitDec(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
const Operator* js_op =
javascript()->Subtract(language_mode(), BinaryOperationHints::Any());
Node* node = NewNode(js_op, environment()->LookupAccumulator(),
jsgraph()->OneConstant());
-
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -1051,12 +1146,12 @@ void BytecodeGraphBuilder::VisitTypeOf(
void BytecodeGraphBuilder::BuildDelete(
const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* key = environment()->LookupAccumulator();
Node* object = environment()->LookupRegister(iterator.GetRegisterOperand(0));
-
Node* node =
NewNode(javascript()->DeleteProperty(language_mode()), object, key);
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -1077,11 +1172,11 @@ void BytecodeGraphBuilder::VisitDeletePropertySloppy(
void BytecodeGraphBuilder::BuildCompareOp(
const Operator* js_op, const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* left = environment()->LookupRegister(iterator.GetRegisterOperand(0));
Node* right = environment()->LookupAccumulator();
Node* node = NewNode(js_op, left, right);
-
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}
@@ -1148,8 +1243,9 @@ void BytecodeGraphBuilder::VisitTestInstanceOf(
void BytecodeGraphBuilder::BuildCastOperator(
const Operator* js_op, const interpreter::BytecodeArrayIterator& iterator) {
+ FrameStateBeforeAndAfter states(this, iterator);
Node* node = NewNode(js_op, environment()->LookupAccumulator());
- AddEmptyFrameStateInputs(node);
+ states.AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
environment()->BindAccumulator(node);
}

Powered by Google App Engine
This is Rietveld 408576698