Skip to content

V
V8
Commit a1ba971c authored by oth's avatar oth Committed by Commit bot
Browse files
Options

[Interpreter] Enable assignments in expressions. 

This change introduces register re-mapping to avoid assignment hazards
in binary expressions. Expressions that cause problems typically have
the form y = x + (x = 4);. The problem occurs because the lhs value
evaluates to the register holding x. The rhs updates that register and
then applying the operation would use the new value as the lhs.

By tracking loads and stores in binary expressions the generator is now
able to detect when condition occurs and uses a temporary register for
the rhs value. When the binary expression evaluation is complete the
variable is updated with the latest temporary.

A new bytecode Mov performs this update without touching the
accumulator.

BUG=v8:4280
LOG=N

Review URL: https://codereview.chromium.org/1412683011

Cr-Commit-Position: refs/heads/master@{#32141}
parent 0ec6db47
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 588 additions and 87 deletions
src/compiler/bytecode-graph-builder.cc
View file @ a1ba971c
......@@ -330,6 +330,13 @@ void BytecodeGraphBuilder::VisitStar(
}
void BytecodeGraphBuilder::VisitMov(
const interpreter::BytecodeArrayIterator& iterator) {
Node* value = environment()->LookupRegister(iterator.GetRegisterOperand(0));
environment()->BindRegister(iterator.GetRegisterOperand(1), value);
}
void BytecodeGraphBuilder::BuildLoadGlobal(
const interpreter::BytecodeArrayIterator& iterator,
TypeofMode typeof_mode) {
......
src/interpreter/bytecode-array-builder.cc
View file @ a1ba971c
......@@ -288,6 +288,13 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::LoadAccumulatorWithRegister(
BytecodeArrayBuilder& BytecodeArrayBuilder::StoreAccumulatorInRegister(
Register reg) {
// TODO(oth): Avoid storing the accumulator in the register if the
// previous bytecode loaded the accumulator with the same register.
//
// TODO(oth): If the previous bytecode is a MOV into this register,
// the previous instruction can be removed. The logic for determining
// these redundant MOVs appears complex.
Output(Bytecode::kStar, reg.ToOperand());
if (!IsRegisterInAccumulator(reg)) {
Output(Bytecode::kStar, reg.ToOperand());
}
......@@ -295,6 +302,14 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreAccumulatorInRegister(
}
BytecodeArrayBuilder& BytecodeArrayBuilder::MoveRegister(Register from,
Register to) {
DCHECK(from != to);
Output(Bytecode::kMov, from.ToOperand(), to.ToOperand());
return *this;
}
BytecodeArrayBuilder& BytecodeArrayBuilder::LoadGlobal(
size_t name_index, int feedback_slot, LanguageMode language_mode,
TypeofMode typeof_mode) {
......
src/interpreter/bytecode-array-builder.h
View file @ a1ba971c
......@@ -97,6 +97,9 @@ class BytecodeArrayBuilder {
BytecodeArrayBuilder& LoadAccumulatorWithRegister(Register reg);
BytecodeArrayBuilder& StoreAccumulatorInRegister(Register reg);
// Register-register transfer.
BytecodeArrayBuilder& MoveRegister(Register from, Register to);
// Named load property.
BytecodeArrayBuilder& LoadNamedProperty(Register object, size_t name_index,
int feedback_slot,
......
src/interpreter/bytecode-generator.cc
View file @ a1ba971c
This diff is collapsed.
src/interpreter/bytecode-generator.h
View file @ a1ba971c
......@@ -13,10 +13,9 @@ namespace v8 {
namespace internal {
namespace interpreter {
class BytecodeGenerator : public AstVisitor {
class BytecodeGenerator final : public AstVisitor {
public:
BytecodeGenerator(Isolate* isolate, Zone* zone);
virtual ~BytecodeGenerator();
Handle<BytecodeArray> MakeBytecode(CompilationInfo* info);
......@@ -36,6 +35,37 @@ class BytecodeGenerator : public AstVisitor {
class EffectResultScope;
class AccumulatorResultScope;
class RegisterResultScope;
class AssignmentHazardScope;
// Helper class that aliases locals and parameters when assignment
// hazards occur in binary expressions. For y = x + (x = 1) has an
// assignment hazard because the lhs evaluates to the register
// holding x and the rhs (x = 1) potentially updates x. When this
// hazard is detected, the rhs uses a temporary to hold the newer
// value of x while preserving the lhs for the binary expresion
// evaluation. The newer value is spilled to x at the end of the
// binary expression evaluation.
class AssignmentHazardHelper final {
public:
explicit AssignmentHazardHelper(BytecodeGenerator* generator);
MUST_USE_RESULT Register GetRegisterForLoad(Register reg);
MUST_USE_RESULT Register GetRegisterForStore(Register reg);
private:
friend class AssignmentHazardScope;
void EnterScope();
void LeaveScope();
void RestoreAliasedLocalsAndParameters();
BytecodeGenerator* generator_;
ZoneMap<int, int> alias_mappings_;
ZoneSet<int> aliased_locals_and_parameters_;
ExpressionResultScope* execution_result_;
int scope_depth_;
DISALLOW_COPY_AND_ASSIGN(AssignmentHazardHelper);
};
void MakeBytecodeBody();
Register NextContextRegister() const;
......@@ -54,6 +84,9 @@ class BytecodeGenerator : public AstVisitor {
void VisitNot(UnaryOperation* expr);
void VisitDelete(UnaryOperation* expr);
// Used by flow control routines to evaluate loop condition.
void VisitCondition(Expression* expr);
// Helper visitors which perform common operations.
Register VisitArguments(ZoneList<Expression*>* arguments);
......@@ -84,17 +117,12 @@ class BytecodeGenerator : public AstVisitor {
Register value_out);
void VisitForInAssignment(Expression* expr, FeedbackVectorSlot slot);
// Visitors for obtaining expression result in the accumulator, in a
// register, or just getting the effect.
void VisitForAccumulatorValue(Expression* expression);
MUST_USE_RESULT Register VisitForRegisterValue(Expression* expression);
void VisitForEffect(Expression* node);
// Methods marking the start and end of binary expressions.
void PrepareForBinaryExpression();
void CompleteBinaryExpression();
// Methods for tracking and remapping register.
void RecordStoreToRegister(Register reg);
Register LoadFromAliasedRegister(Register reg);
......@@ -121,6 +149,9 @@ class BytecodeGenerator : public AstVisitor {
execution_result_ = execution_result;
}
ExpressionResultScope* execution_result() const { return execution_result_; }
inline AssignmentHazardHelper* assignment_hazard_helper() {
return &assignment_hazard_helper_;
}
ZoneVector<Handle<Object>>* globals() { return &globals_; }
inline LanguageMode language_mode() const;
......@@ -136,9 +167,7 @@ class BytecodeGenerator : public AstVisitor {
ControlScope* execution_control_;
ContextScope* execution_context_;
ExpressionResultScope* execution_result_;
int binary_expression_depth_;
ZoneSet<int> binary_expression_hazard_set_;
AssignmentHazardHelper assignment_hazard_helper_;
};
} // namespace interpreter
......
src/interpreter/bytecodes.h
View file @ a1ba971c
......@@ -70,6 +70,9 @@ namespace interpreter {
V(Ldar, OperandType::kReg8) \
V(Star, OperandType::kReg8) \
\
/* Register-register transfers */ \
V(Mov, OperandType::kReg8, OperandType::kReg8) \
\
/* LoadIC operations */ \
V(LoadICSloppy, OperandType::kReg8, OperandType::kIdx8, OperandType::kIdx8) \
V(LoadICStrict, OperandType::kReg8, OperandType::kIdx8, OperandType::kIdx8) \
......
src/interpreter/interpreter.cc
View file @ a1ba971c
......@@ -200,6 +200,18 @@ void Interpreter::DoStar(compiler::InterpreterAssembler* assembler) {
}
// Mov <src> <dst>
//
// Stores the value of register <src> to register <dst>.
void Interpreter::DoMov(compiler::InterpreterAssembler* assembler) {
Node* src_index = __ BytecodeOperandReg(0);
Node* src_value = __ LoadRegister(src_index);
Node* dst_index = __ BytecodeOperandReg(1);
__ StoreRegister(src_value, dst_index);
__ Dispatch();
}
void Interpreter::DoLoadGlobal(Callable ic,
compiler::InterpreterAssembler* assembler) {
// Get the global object.
......@@ -216,7 +228,6 @@ void Interpreter::DoLoadGlobal(Callable ic,
Node* result = __ CallIC(ic.descriptor(), code_target, global, name, smi_slot,
type_feedback_vector);
__ SetAccumulator(result);
__ Dispatch();
}
......
test/cctest/interpreter/test-bytecode-generator.cc
View file @ a1ba971c
......@@ -5264,6 +5264,225 @@ TEST(RemoveRedundantLdar) {
}
}
TEST(AssignmentsInBinaryExpression) {
InitializedHandleScope handle_scope;
BytecodeGeneratorHelper helper;
ExpectedSnippet<const char*> snippets[] = {
{"var x = 0, y = 1;\n"
"return (x = 2, y = 3, x = 4, y = 5)",
2 * kPointerSize,
1,
24,
{
B(LdaZero), B(Star), R(0), //
B(LdaSmi8), U8(1), //
B(Star), R(1), //
B(LdaSmi8), U8(2), //
B(Star), R(0), //
B(LdaSmi8), U8(3), //
B(Star), R(1), //
B(LdaSmi8), U8(4), //
B(Star), R(0), //
B(LdaSmi8), U8(5), //
B(Star), R(1), //
B(Return), //
},
0},
{"var x = 55;\n"
"var y = (x = 100);\n"
"return y",
2 * kPointerSize,
1,
11,
{
B(LdaSmi8), U8(55), //
B(Star), R(0), //
B(LdaSmi8), U8(100), //
B(Star), R(0), //
B(Star), R(1), //
B(Return), //
},
0},
{"var x = 55;\n"
"x = x + (x = 100) + (x = 101);\n"
"return x;",
4 * kPointerSize,
1,
24,
{
B(LdaSmi8), U8(55), //
B(Star), R(0), //
B(LdaSmi8), U8(100), //
B(Star), R(1), //
B(Add), R(0), //
B(Star), R(2), //
B(LdaSmi8), U8(101), //
B(Star), R(3), //
B(Add), R(2), //
B(Mov), R(3), R(0), //
B(Star), R(0), //
B(Return), //
},
0},
{"var x = 55;\n"
"x = (x = 56) - x + (x = 57);\n"
"x++;\n"
"return x;",
3 * kPointerSize,
1,
34,
{
B(LdaSmi8), U8(55), //
B(Star), R(0), //
B(LdaSmi8), U8(56), //
B(Star), R(0), //
B(Star), R(1), //
B(Ldar), R(0), //
B(Sub), R(1), //
B(Star), R(2), //
B(LdaSmi8), U8(57), //
B(Star), R(1), //
B(Add), R(2), //
B(Mov), R(1), R(0), //
B(Star), R(0), //
B(ToNumber), //
B(Star), R(1), //
B(Inc), //
B(Star), R(0), //
B(Return), //
},
0},
{"var x = 55;\n"
"var y = x + (x = 1) + (x = 2) + (x = 3);\n"
"return y;",
6 * kPointerSize,
1,
32,
{
B(LdaSmi8), U8(55), //
B(Star), R(0), //
B(LdaSmi8), U8(1), //
B(Star), R(2), //
B(Add), R(0), //
B(Star), R(3), //
B(LdaSmi8), U8(2), //
B(Star), R(4), //
B(Add), R(3), //
B(Star), R(5), //
B(LdaSmi8), U8(3), //
B(Star), R(3), //
B(Add), R(5), //
B(Mov), R(3), R(0), //
B(Star), R(1), //
B(Return), //
},
0},
{"var x = 55;\n"
"var x = x + (x = 1) + (x = 2) + (x = 3);\n"
"return x;",
5 * kPointerSize,
1,
32,
{
B(LdaSmi8), U8(55), //
B(Star), R(0), //
B(LdaSmi8), U8(1), //
B(Star), R(1), //
B(Add), R(0), //
B(Star), R(2), //
B(LdaSmi8), U8(2), //
B(Star), R(3), //
B(Add), R(2), //
B(Star), R(4), //
B(LdaSmi8), U8(3), //
B(Star), R(2), //
B(Add), R(4), //
B(Mov), R(2), R(0), //
B(Star), R(0), //
B(Return), //
},
0},
{"var x = 10, y = 20;\n"
"return x + (x = 1) + (x + 1) * (y = 2) + (y = 3) + (x = 4) + (y = 5) + "
"y;\n",
6 * kPointerSize,
1,
64,
{
B(LdaSmi8), U8(10), //
B(Star), R(0), //
B(LdaSmi8), U8(20), //
B(Star), R(1), //
B(LdaSmi8), U8(1), //
B(Star), R(2), //
B(Add), R(0), //
B(Star), R(3), //
B(LdaSmi8), U8(1), //
B(Add), R(2), //
B(Star), R(4), //
B(LdaSmi8), U8(2), //
B(Star), R(1), //
B(Mul), R(4), //
B(Add), R(3), //
B(Star), R(4), //
B(LdaSmi8), U8(3), //
B(Star), R(1), //
B(Add), R(4), //
B(Star), R(3), //
B(LdaSmi8), U8(4), //
B(Star), R(4), //
B(Add), R(3), //
B(Star), R(5), //
B(LdaSmi8), U8(5), //
B(Star), R(1), //
B(Add), R(5), //
B(Star), R(3), //
B(Ldar), R(1), //
B(Add), R(3), //
B(Mov), R(4), R(0), //
B(Return), //
},
0},
{"var x = 17;\n"
"return 1 + x + (x++) + (++x);\n",
5 * kPointerSize,
1,
40,
{
B(LdaSmi8), U8(17), //
B(Star), R(0), //
B(LdaSmi8), U8(1), //
B(Star), R(1), //
B(Ldar), R(0), //
B(Add), R(1), //
B(Star), R(2), //
B(Ldar), R(0), //
B(ToNumber), //
B(Star), R(1), //
B(Inc), //
B(Star), R(3), //
B(Ldar), R(1), //
B(Add), R(2), //
B(Star), R(4), //
B(Ldar), R(3), //
B(ToNumber), //
B(Inc), //
B(Star), R(1), //
B(Add), R(4), //
B(Mov), R(1), R(0), //
B(Return), //
},
0}};
for (size_t i = 0; i < arraysize(snippets); i++) {
Handle<BytecodeArray> bytecode_array =
helper.MakeBytecodeForFunctionBody(snippets[i].code_snippet);
CheckBytecodeArrayEqual(snippets[i], bytecode_array);
}
}
} // namespace interpreter
} // namespace internal
} // namespace v8
test/cctest/interpreter/test-interpreter.cc
View file @ a1ba971c
......@@ -2617,7 +2617,6 @@ TEST(InterpreterBasicLoops) {
TEST(InterpreterForIn) {
HandleAndZoneScope handles;
// TODO(oth): Add a test here for delete mid-loop when delete is ready.
std::pair<const char*, int> for_in_samples[] = {
{"function f() {\n"
" var r = -1;\n"
......@@ -2950,6 +2949,148 @@ TEST(InterpreterNewTarget) {
CHECK(new_target_name->SameValue(*factory->NewStringFromStaticChars("f")));
}
TEST(InterpreterAssignmentInExpressions) {
HandleAndZoneScope handles;
std::pair<const char*, int> samples[] = {
{"function f() {\n"
" var x = 7;\n"
" var y = x + (x = 1) + (x = 2);\n"
" return y;\n"
"}",
10},
{"function f() {\n"
" var x = 7;\n"
" var y = x + (x = 1) + (x = 2);\n"
" return x;\n"
"}",
2},
{"function f() {\n"
" var x = 55;\n"
" x = x + (x = 100) + (x = 101);\n"
" return x;\n"
"}",
256},
{"function f() {\n"
" var x = 7;\n"
" return ++x + x + x++;\n"
"}",
24},
{"function f() {\n"
" var x = 7;\n"
" var y = 1 + ++x + x + x++;\n"
" return x;\n"
"}",
9},
{"function f() {\n"
" var x = 7;\n"
" var y = ++x + x + x++;\n"
" return x;\n"
"}",
9},
{"function f() {\n"
" var x = 7, y = 100, z = 1000;\n"
" return x + (x += 3) + y + (y *= 10) + (z *= 7) + z;\n"
"}",
15117},
{"function f() {\n"
" var inner = function (x) { return x + (x = 2) + (x = 4) + x; };\n"
" return inner(1);\n"
"}",
11},
{"function f() {\n"
" var x = 1, y = 2;\n"
" x = x + (x = 3) + y + (y = 4), y = y + (y = 5) + y + x;\n"
" return x + y;\n"
"}",
10 + 24},
{"function f() {\n"
" var x = 0;\n"
" var y = x | (x = 1) | (x = 2);\n"
" return x;\n"
"}",
2},
{"function f() {\n"
" var x = 0;\n"
" var y = x || (x = 1);\n"
" return x;\n"
"}",
1},
{"function f() {\n"
" var x = 1;\n"
" var y = x && (x = 2) && (x = 3);\n"
" return x;\n"
"}",
3},
{"function f() {\n"
" var x = 1;\n"
" var y = x || (x = 2);\n"
" return x;\n"
"}",
1},
{"function f() {\n"
" var x = 1;\n"
" x = (x << (x = 3)) | (x = 16);\n"
" return x;\n"
"}",
24},
{"function f() {\n"
" var r = 7;\n"
" var s = 11;\n"
" var t = 13;\n"
" var u = r + s + t + (r = 10) + (s = 20) +"
" (t = (r + s)) + r + s + t;\n"
" return r + s + t + u;\n"
"}",
211},
{"function f() {\n"
" var r = 7;\n"
" var s = 11;\n"
" var t = 13;\n"
" return r > (3 * s * (s = 1)) ? (t + (t += 1)) : (r + (r = 4));\n"
"}",
11},
{"function f() {\n"
" var r = 7;\n"
" var s = 11;\n"
" var t = 13;\n"
" return r > (3 * s * (s = 0)) ? (t + (t += 1)) : (r + (r = 4));\n"
"}",
27},
{"function f() {\n"
" var r = 7;\n"
" var s = 11;\n"
" var t = 13;\n"
" return (r + (r = 5)) > s ? r : t;\n"
"}",
5},
{"function f(a) {\n"
" return a + (arguments[0] = 10);\n"
"}",
50},
{"function f(a) {\n"
" return a + (arguments[0] = 10) + a;\n"
"}",
60},
{"function f(a) {\n"
" return a + (arguments[0] = 10) + arguments[0];\n"
"}",
60},
};
const int arg_value = 40;
for (size_t i = 0; i < arraysize(samples); i++) {
InterpreterTester tester(handles.main_isolate(), samples[i].first);
auto callable = tester.GetCallable<Handle<Object>>();
Handle<Object> return_val =
callable(handle(Smi::FromInt(arg_value), handles.main_isolate()))
.ToHandleChecked();
CHECK_EQ(Handle<Smi>::cast(return_val)->value(), samples[i].second);
}
}
} // namespace interpreter
} // namespace internal
} // namespace v8
test/unittests/interpreter/bytecode-array-builder-unittest.cc
View file @ a1ba971c
......@@ -22,12 +22,12 @@ class BytecodeArrayBuilderTest : public TestWithIsolateAndZone {
TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
BytecodeArrayBuilder builder(isolate(), zone());
builder.set_locals_count(1);
builder.set_locals_count(2);
builder.set_context_count(1);
builder.set_parameter_count(0);
CHECK_EQ(builder.locals_count(), 1);
CHECK_EQ(builder.locals_count(), 2);
CHECK_EQ(builder.context_count(), 1);
CHECK_EQ(builder.fixed_register_count(), 2);
CHECK_EQ(builder.fixed_register_count(), 3);
// Emit constant loads.
builder.LoadLiteral(Smi::FromInt(0))
......@@ -46,6 +46,10 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
.LoadNull()
.StoreAccumulatorInRegister(reg);
// Emit register-register transfer.
Register other(1);
builder.MoveRegister(reg, other);
// Emit global load / store operations.
builder.LoadGlobal(0, 1, LanguageMode::SLOPPY, TypeofMode::NOT_INSIDE_TYPEOF)
.LoadGlobal(0, 1, LanguageMode::STRICT, TypeofMode::NOT_INSIDE_TYPEOF)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment