Skip to content

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

Work around issue 260 for now by disabling duplication of the loop 

condition for while and for loops.

http://code.google.com/p/v8/issues/detail?id=260

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1453 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
parent 9c96c68a
Show whitespace changes
Inline Side-by-side
Showing with 141 additions and 57 deletions
src/codegen-ia32.cc
View file @ 34af9f2e
......@@ -2172,15 +2172,16 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
JumpTarget body(this, JumpTarget::BIDIRECTIONAL);
IncrementLoopNesting();
// Label the top of the loop for the backward CFG edge. If the test
// is always true we can use the continue target, and if the test is
// always false there is no need.
// Label the top of the loop for the backward jump if necessary.
if (info == ALWAYS_TRUE) {
// Use the continue target.
node->continue_target()->Initialize(this, JumpTarget::BIDIRECTIONAL);
node->continue_target()->Bind();
} else if (info == ALWAYS_FALSE) {
// No need to label it.
node->continue_target()->Initialize(this);
} else {
// Continue is the test, so use the backward body target.
ASSERT(info == DONT_KNOW);
node->continue_target()->Initialize(this);
body.Bind();
......@@ -2191,8 +2192,8 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
// Compile the test.
if (info == ALWAYS_TRUE) {
// If control flow can fall off the end of the body, jump back to
// the top and bind the break target as the exit.
// If control flow can fall off the end of the body, jump back
// to the top and bind the break target at the exit.
if (has_valid_frame()) {
node->continue_target()->Jump();
}
......@@ -2228,15 +2229,19 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
}
case LoopStatement::WHILE_LOOP: {
JumpTarget body(this, JumpTarget::BIDIRECTIONAL);
// TODO(260): This flag controls whether to duplicate the test
// at the bottom of the loop. Replace it with a better
// indication of when it is safe to do so.
static const bool test_at_bottom = false;
JumpTarget body; // Uninitialized.
IncrementLoopNesting();
// If the condition is always false and has no side effects, we
// do not need to compile anything.
if (info == ALWAYS_FALSE) break;
// Based on the condition analysis, compile the test if
// necessary and label the body if necessary.
// Based on the condition analysis, compile the test as necessary.
if (info == ALWAYS_TRUE) {
// We will not compile the test expression. Label the top of
// the loop with the continue target.
......@@ -2244,7 +2249,18 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
node->continue_target()->Bind();
} else {
ASSERT(info == DONT_KNOW); // ALWAYS_FALSE cannot reach here.
if (test_at_bottom) {
// Continue is the test at the bottom, no need to label the
// test at the top. The body is a backward target.
node->continue_target()->Initialize(this);
body.Initialize(this, JumpTarget::BIDIRECTIONAL);
} else {
// Label the test at the top as the continue target. The
// body is a forward-only target.
node->continue_target()->Initialize(this, JumpTarget::BIDIRECTIONAL);
node->continue_target()->Bind();
body.Initialize(this);
}
// Compile the test with the body as the true target and
// preferred fall-through and with the break target as the
// false target.
......@@ -2252,9 +2268,9 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
LoadCondition(node->cond(), NOT_INSIDE_TYPEOF, &dest, true);
if (dest.false_was_fall_through()) {
// If we don't have dangling jumps to the body, the test is
// unconditionally false and we do not need to compile the
// body.
// If we got the break target as fall-through, the test may
// have been unconditionally false (if there are no jumps to
// the body).
if (!body.is_linked()) break;
// Otherwise, jump around the body on the fall through and
......@@ -2265,31 +2281,37 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
}
}
// The (stack check at the start of the) body was labeled.
// Compile it.
CheckStack(); // TODO(1222600): ignore if body contains calls.
Visit(node->body());
// Compile the test if necessary and jump back.
// Based on the condition analysis, compile the backward jump as
// necessary.
if (info == ALWAYS_TRUE) {
// The body has been labeled with the continue target.
// The loop body has been labeled with the continue target.
if (has_valid_frame()) {
node->continue_target()->Jump();
}
} else {
ASSERT(info == DONT_KNOW); // ALWAYS_FALSE cannot reach here.
if (test_at_bottom) {
// If we have chosen to recompile the test at the bottom,
// then it is the continue target.
if (node->continue_target()->is_linked()) {
node->continue_target()->Bind();
}
// If control can reach the bottom by falling off the body or
// a continue in the body, (re)compile the test at the bottom.
if (has_valid_frame()) {
// The break target is the fall-through (body is a backward
// jump from here).
// jump from here and thus an invalid fall-through).
ControlDestination dest(&body, node->break_target(), false);
LoadCondition(node->cond(), NOT_INSIDE_TYPEOF, &dest, true);
}
} else {
// If we have chosen not to recompile the test at the
// bottom, jump back to the one at the top.
if (has_valid_frame()) {
node->continue_target()->Jump();
}
}
}
// The break target may be already bound (by the condition), or
......@@ -2301,7 +2323,13 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
}
case LoopStatement::FOR_LOOP: {
JumpTarget body(this, JumpTarget::BIDIRECTIONAL);
// TODO(260): This flag controls whether to duplicate the test
// at the bottom of the loop. Replace it with a better
// indication of when it is safe to do so.
static const bool test_at_bottom = false;
JumpTarget loop(this, JumpTarget::BIDIRECTIONAL);
JumpTarget body(this);
// Compile the init expression if present.
if (node->init() != NULL) {
......@@ -2314,22 +2342,37 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
// do not need to compile anything else.
if (info == ALWAYS_FALSE) break;
// Based on the condition analysis, compile the test if
// necessary and label the body if necessary.
// Based on the condition analysis, compile the test as necessary.
if (info == ALWAYS_TRUE) {
// We will not compile the test expression. Label the top of
// the loop with the continue target if there is no update
// expression, otherwise with the body target.
// the loop.
if (node->next() == NULL) {
// Use the continue target if there is no update expression.
node->continue_target()->Initialize(this, JumpTarget::BIDIRECTIONAL);
node->continue_target()->Bind();
} else {
// Otherwise use the backward loop target.
node->continue_target()->Initialize(this);
body.Bind();
loop.Bind();
}
} else {
ASSERT(info == DONT_KNOW);
if (test_at_bottom) {
// Continue is either the update expression or the test at
// the bottom, no need to label the test at the top.
node->continue_target()->Initialize(this);
} else if (node->next() == NULL) {
// We are not recompiling the test at the bottom and there
// is no update expression.
node->continue_target()->Initialize(this, JumpTarget::BIDIRECTIONAL);
node->continue_target()->Bind();
} else {
// We are not recompiling the test at the bottom and there
// is an update expression.
node->continue_target()->Initialize(this);
loop.Bind();
}
// Compile the test with the body as the true target and
// preferred fall-through and with the break target as the
// false target.
......@@ -2337,9 +2380,9 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
LoadCondition(node->cond(), NOT_INSIDE_TYPEOF, &dest, true);
if (dest.false_was_fall_through()) {
// If we don't have dangling jumps to the body, the test is
// unconditionally false and we do not need to compile the
// body.
// If we got the break target as fall-through, the test may
// have been unconditionally false (if there are no jumps to
// the body).
if (!body.is_linked()) break;
// Otherwise, jump around the body on the fall through and
......@@ -2350,20 +2393,17 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
}
}
// The (stack check at the start of the) body was labeled.
// Compile it.
CheckStack(); // TODO(1222600): ignore if body contains calls.
Visit(node->body());
// If there is an update expression, compile it if necessary.
if (node->next() != NULL) {
// We did not use the continue target for the body.
if (node->continue_target()->is_linked()) {
node->continue_target()->Bind();
}
// Control can reach the update by falling out of the body or
// by a continue in the body.
// by a continue.
if (has_valid_frame()) {
// Record the source position of the statement as this code
// which is after the code for the body actually belongs to
......@@ -2373,32 +2413,43 @@ void CodeGenerator::VisitLoopStatement(LoopStatement* node) {
}
}
// Compile the test if necessary and jump back.
// Based on the condition analysis, compile the backward jump as
// necessary.
if (info == ALWAYS_TRUE) {
if (has_valid_frame()) {
if (node->next() == NULL) {
node->continue_target()->Jump();
} else {
body.Jump();
loop.Jump();
}
}
} else {
ASSERT(info == DONT_KNOW); // ALWAYS_FALSE cannot reach here.
if (test_at_bottom) {
if (node->continue_target()->is_linked()) {
// We can have dangling jumps to the continue target if
// there was no update expression.
node->continue_target()->Bind();
}
// Control can reach the test at the bottom by falling out of
// the body, by a continue in the body, or from the update
// expression.
// Control can reach the test at the bottom by falling out
// of the body, by a continue in the body, or from the
// update expression.
if (has_valid_frame()) {
// The break target is the fall-through (body is a backward
// jump from here).
// The break target is the fall-through (body is a
// backward jump from here).
ControlDestination dest(&body, node->break_target(), false);
LoadCondition(node->cond(), NOT_INSIDE_TYPEOF, &dest, true);
}
} else {
// Otherwise, jump back to the test at the top.
if (has_valid_frame()) {
if (node->next() == NULL) {
node->continue_target()->Jump();
} else {
loop.Jump();
}
}
}
}
// The break target may be already bound (by the condition), or
......
test/mjsunit/regress/regress-260.js 0 → 100644
View file @ 34af9f2e
// Copyright 2009 the V8 project authors. All rights reserved.
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following
// disclaimer in the documentation and/or other materials provided
// with the distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived
// from this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// We should not compile the bodies of function literals in loop
// conditions twice, even in cases where the loop condition is
// compiled twice.
function test() { eval("while(!function () { var x; });"); }
test();
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