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

Unified Diff: src/codegen-ia32.cc

Issue 42127: Reenable the duplicated test at the bottom of for and while loops. It... (Closed) Base URL: http://v8.googlecode.com/svn/branches/bleeding_edge/
Patch Set: Created 11 years, 9 months 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
« src/ast.h ('K') | « src/ast.h ('k') | src/jump-target.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/codegen-ia32.cc
===================================================================
--- src/codegen-ia32.cc (revision 1501)
+++ src/codegen-ia32.cc (working copy)
@@ -2228,18 +2228,24 @@
}
case LoopStatement::WHILE_LOOP: {
- // 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;
+ // Do not duplicate conditions with function literal
+ // subexpressions. This can cause us to compile the function
+ // literal twice.
+ bool test_at_bottom = !node->has_function_literal();
- JumpTarget body(this); // Initialized as forward-only.
IncrementLoopNesting();
// If the condition is always false and has no side effects, we
// do not need to compile anything.
if (info == ALWAYS_FALSE) break;
+ JumpTarget body;
+ if (test_at_bottom) {
+ body.Initialize(this, JumpTarget::BIDIRECTIONAL);
+ } else {
+ body.Initialize(this);
+ }
+
// 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
@@ -2252,7 +2258,6 @@
// 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.make_bidirectional();
} else {
// Label the test at the top as the continue target. The
// body is a forward-only target.
@@ -2321,14 +2326,11 @@
}
case LoopStatement::FOR_LOOP: {
- // 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;
+ // Do not duplicate conditions with function literal
+ // subexpressions. This can cause us to compile the function
+ // literal twice.
+ bool test_at_bottom = !node->has_function_literal();
- JumpTarget loop(this, JumpTarget::BIDIRECTIONAL);
- JumpTarget body(this);
-
// Compile the init expression if present.
if (node->init() != NULL) {
Visit(node->init());
@@ -2340,6 +2342,19 @@
// do not need to compile anything else.
if (info == ALWAYS_FALSE) break;
+ // Target for backward edge if no test at the bottom, otherwise
+ // unused.
+ JumpTarget loop(this, JumpTarget::BIDIRECTIONAL);
+
+ // Target for backward edge if there is a test at the bottom,
+ // otherwise used as target for test at the top.
+ JumpTarget body;
+ if (test_at_bottom) {
+ body.Initialize(this, JumpTarget::BIDIRECTIONAL);
+ } else {
+ body.Initialize(this);
+ }
+
// 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
« src/ast.h ('K') | « src/ast.h ('k') | src/jump-target.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698