Skip to content

V
V8
Commit 35c85237 authored by kmillikin@chromium.org's avatar kmillikin@chromium.org
Browse files
Options

Fix a bug in the short-circuit logical operations in the toplevel 

code generator.

Review URL: http://codereview.chromium.org/542105

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3636 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
parent 37d39724
Show whitespace changes
Inline Side-by-side
Showing with 221 additions and 41 deletions
src/ia32/fast-codegen-ia32.cc
View file @ 35c85237
......@@ -214,17 +214,24 @@ void FastCodeGenerator::Apply(Expression::Context context, Register reg) {
}
break;
case Expression::kValueTest:
case Expression::kTestValue:
// Push an extra copy of the value in case it's needed.
__ push(reg);
// Fall through.
case Expression::kTest:
// For simplicity we always test the accumulator register.
if (!reg.is(result_register())) __ mov(result_register(), reg);
DoTest(context);
break;
case Expression::kValueTest:
case Expression::kTestValue:
if (!reg.is(result_register())) __ mov(result_register(), reg);
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
}
......@@ -251,6 +258,7 @@ void FastCodeGenerator::Apply(Expression::Context context, Slot* slot) {
}
case Expression::kTest:
// For simplicity we always test the accumulator register.
Move(result_register(), slot);
DoTest(context);
break;
......@@ -258,7 +266,13 @@ void FastCodeGenerator::Apply(Expression::Context context, Slot* slot) {
case Expression::kValueTest:
case Expression::kTestValue:
Move(result_register(), slot);
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
......@@ -285,6 +299,7 @@ void FastCodeGenerator::Apply(Expression::Context context, Literal* lit) {
break;
case Expression::kTest:
// For simplicity we always test the accumulator register.
__ mov(result_register(), lit->handle());
DoTest(context);
break;
......@@ -292,7 +307,13 @@ void FastCodeGenerator::Apply(Expression::Context context, Literal* lit) {
case Expression::kValueTest:
case Expression::kTestValue:
__ mov(result_register(), lit->handle());
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
......@@ -319,13 +340,21 @@ void FastCodeGenerator::ApplyTOS(Expression::Context context) {
break;
case Expression::kTest:
// For simplicity we always test the accumulator register.
__ pop(result_register());
DoTest(context);
break;
case Expression::kValueTest:
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
__ pop(result_register());
break;
case kStack:
__ mov(result_register(), Operand(esp, 0));
break;
}
DoTest(context);
break;
}
......@@ -359,6 +388,7 @@ void FastCodeGenerator::DropAndApply(int count,
break;
case Expression::kTest:
// For simplicity we always test the accumulator register.
__ Drop(count);
if (!reg.is(result_register())) __ mov(result_register(), reg);
DoTest(context);
......@@ -366,9 +396,17 @@ void FastCodeGenerator::DropAndApply(int count,
case Expression::kValueTest:
case Expression::kTestValue:
if (count > 1) __ Drop(count - 1);
switch (location_) {
case kAccumulator:
__ Drop(count);
if (!reg.is(result_register())) __ mov(result_register(), reg);
break;
case kStack:
if (count > 1) __ Drop(count - 1);
__ mov(result_register(), reg);
__ mov(Operand(esp, 0), result_register());
break;
}
DoTest(context);
break;
}
......@@ -440,18 +478,44 @@ void FastCodeGenerator::Apply(Expression::Context context,
void FastCodeGenerator::DoTest(Expression::Context context) {
// The value to test is in the accumulator, and duplicated on the stack if
// necessary (for value/test and test/value contexts).
// The value to test is in the accumulator. If the value might be needed
// on the stack (value/test and test/value contexts with a stack location
// desired), then the value is already duplicated on the stack.
ASSERT_NE(NULL, true_label_);
ASSERT_NE(NULL, false_label_);
// If there is a value on the stack, use a discard label for the
// value-is-unneeded branch in the inlined part of the test.
// In value/test and test/value expression contexts with stack as the
// desired location, there is already an extra value on the stack. Use a
// label to discard it if unneeded.
Label discard;
Label* if_true =
(context == Expression::kTestValue) ? &discard : true_label_;
Label* if_false =
(context == Expression::kValueTest) ? &discard : false_label_;
Label* if_true = true_label_;
Label* if_false = false_label_;
switch (context) {
case Expression::kUninitialized:
case Expression::kEffect:
case Expression::kValue:
UNREACHABLE();
case Expression::kTest:
break;
case Expression::kValueTest:
switch (location_) {
case kAccumulator:
break;
case kStack:
if_false = &discard;
break;
}
break;
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
break;
case kStack:
if_true = &discard;
break;
}
break;
}
// Emit the inlined tests assumed by the stub.
__ cmp(result_register(), Factory::undefined_value());
......@@ -466,6 +530,34 @@ void FastCodeGenerator::DoTest(Expression::Context context) {
__ test(result_register(), Immediate(kSmiTagMask));
__ j(zero, if_true);
// Save a copy of the value if it may be needed and isn't already saved.
switch (context) {
case Expression::kUninitialized:
case Expression::kEffect:
case Expression::kValue:
UNREACHABLE();
case Expression::kTest:
break;
case Expression::kValueTest:
switch (location_) {
case kAccumulator:
__ push(result_register());
break;
case kStack:
break;
}
break;
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
__ push(result_register());
break;
case kStack:
break;
}
break;
}
// Call the ToBoolean stub for all other cases.
ToBooleanStub stub;
__ push(result_register());
......
src/x64/fast-codegen-x64.cc
View file @ 35c85237
......@@ -221,17 +221,24 @@ void FastCodeGenerator::Apply(Expression::Context context, Register reg) {
}
break;
case Expression::kValueTest:
case Expression::kTestValue:
// Push an extra copy of the value in case it's needed.
__ push(reg);
// Fall through.
case Expression::kTest:
// For simplicity we always test the accumulator register.
if (!reg.is(result_register())) __ movq(result_register(), reg);
DoTest(context);
break;
case Expression::kValueTest:
case Expression::kTestValue:
if (!reg.is(result_register())) __ movq(result_register(), reg);
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
}
......@@ -265,7 +272,13 @@ void FastCodeGenerator::Apply(Expression::Context context, Slot* slot) {
case Expression::kValueTest:
case Expression::kTestValue:
Move(result_register(), slot);
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
......@@ -298,7 +311,13 @@ void FastCodeGenerator::Apply(Expression::Context context, Literal* lit) {
case Expression::kValueTest:
case Expression::kTestValue:
__ Move(result_register(), lit->handle());
switch (location_) {
case kAccumulator:
break;
case kStack:
__ push(result_register());
break;
}
DoTest(context);
break;
}
......@@ -331,7 +350,14 @@ void FastCodeGenerator::ApplyTOS(Expression::Context context) {
case Expression::kValueTest:
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
__ pop(result_register());
break;
case kStack:
__ movq(result_register(), Operand(rsp, 0));
break;
}
DoTest(context);
break;
}
......@@ -372,9 +398,17 @@ void FastCodeGenerator::DropAndApply(int count,
case Expression::kValueTest:
case Expression::kTestValue:
if (count > 1) __ Drop(count - 1);
switch (location_) {
case kAccumulator:
__ Drop(count);
if (!reg.is(result_register())) __ movq(result_register(), reg);
break;
case kStack:
if (count > 1) __ Drop(count - 1);
__ movq(result_register(), reg);
__ movq(Operand(rsp, 0), result_register());
break;
}
DoTest(context);
break;
}
......@@ -446,18 +480,44 @@ void FastCodeGenerator::Apply(Expression::Context context,
void FastCodeGenerator::DoTest(Expression::Context context) {
// The value to test is in the accumulator, and duplicated on the stack if
// necessary (for value/test and test/value contexts).
// The value to test is in the accumulator. If the value might be needed
// on the stack (value/test and test/value contexts with a stack location
// desired), then the value is already duplicated on the stack.
ASSERT_NE(NULL, true_label_);
ASSERT_NE(NULL, false_label_);
// If there is a value on the stack, use a discard label for the
// value-is-unneeded branch in the inlined part of the test.
// In value/test and test/value expression contexts with stack as the
// desired location, there is already an extra value on the stack. Use a
// label to discard it if unneeded.
Label discard;
Label* if_true =
(context == Expression::kTestValue) ? &discard : true_label_;
Label* if_false =
(context == Expression::kValueTest) ? &discard : false_label_;
Label* if_true = true_label_;
Label* if_false = false_label_;
switch (context) {
case Expression::kUninitialized:
case Expression::kEffect:
case Expression::kValue:
UNREACHABLE();
case Expression::kTest:
break;
case Expression::kValueTest:
switch (location_) {
case kAccumulator:
break;
case kStack:
if_false = &discard;
break;
}
break;
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
break;
case kStack:
if_true = &discard;
break;
}
break;
}
// Emit the inlined tests assumed by the stub.
__ CompareRoot(result_register(), Heap::kUndefinedValueRootIndex);
......@@ -472,6 +532,34 @@ void FastCodeGenerator::DoTest(Expression::Context context) {
Condition is_smi = masm_->CheckSmi(result_register());
__ j(is_smi, if_true);
// Save a copy of the value if it may be needed and isn't already saved.
switch (context) {
case Expression::kUninitialized:
case Expression::kEffect:
case Expression::kValue:
UNREACHABLE();
case Expression::kTest:
break;
case Expression::kValueTest:
switch (location_) {
case kAccumulator:
__ push(result_register());
break;
case kStack:
break;
}
break;
case Expression::kTestValue:
switch (location_) {
case kAccumulator:
__ push(result_register());
break;
case kStack:
break;
}
break;
}
// Call the ToBoolean stub for all other cases.
ToBooleanStub stub;
__ push(result_register());
......
src/x64/virtual-frame-x64.h
View file @ 35c85237
......@@ -343,7 +343,7 @@ class VirtualFrame : public ZoneObject {
// of the frame. Key and receiver are not dropped.
Result CallKeyedStoreIC();
// Call call IC. Arguments, reciever, and function name are found
// Call call IC. Arguments, receiver, and function name are found
// on top of the frame. Function name slot is not dropped. The
// argument count does not include the receiver.
Result CallCallIC(RelocInfo::Mode mode, int arg_count, int loop_nesting);
......
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