Reviewers: fschneider, Kevin Millikin,
http://codereview.chromium.org/7046073/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/7046073/diff/1/src/hydrogen.cc#newcode1991
src/hydrogen.cc:1991: test_context_ = new TestContext(owner, cond,
if_true, if_false);
Is this part of the change correct? I'm only 50% sure... :-/
Description:
First steps towards better code generation for LBranch:
* AST Expression nodes get a separate testing ID to record type info in
ToBooleanStub later. This is necessary to avoid clashes with other uses
of
already existing IDs.
* In order to avoid threading the condition expression through tons of
places,
TestContexts carry it now with them. Note that we will probably only
need the
testing ID of the expression, but having the whole thing at hand makes
debugging easier. Probably we will change this later...
Please review this at http://codereview.chromium.org/7046073/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/arm/full-codegen-arm.cc
M src/ast.h
M src/full-codegen.h
M src/full-codegen.cc
M src/hydrogen.h
M src/hydrogen.cc
M src/ia32/full-codegen-ia32.cc
M src/x64/full-codegen-x64.cc
Index: src/arm/full-codegen-arm.cc
===================================================================
--- src/arm/full-codegen-arm.cc (revision 8228)
+++ src/arm/full-codegen-arm.cc (working copy)
@@ -383,7 +383,7 @@
// For simplicity we always test the accumulator register.
codegen()->Move(result_register(), slot);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -417,7 +417,7 @@
if (true_label_ != fall_through_) __ b(true_label_);
} else {
__ LoadRoot(result_register(), index);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_,
fall_through_);
}
}
@@ -464,7 +464,7 @@
} else {
// For simplicity we always test the accumulator register.
__ mov(result_register(), Operand(lit));
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_,
fall_through_);
}
}
@@ -500,7 +500,7 @@
__ Drop(count);
__ Move(result_register(), reg);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -578,7 +578,8 @@
}
-void FullCodeGenerator::DoTest(Label* if_true,
+void FullCodeGenerator::DoTest(Expression* condition,
+ Label* if_true,
Label* if_false,
Label* fall_through) {
if (CpuFeatures::IsSupported(VFP3)) {
Index: src/ast.h
===================================================================
--- src/ast.h (revision 8228)
+++ src/ast.h (working copy)
@@ -210,7 +210,7 @@
kTest
};
- Expression() : id_(GetNextId()) {}
+ Expression() : id_(GetNextId()), test_id_(GetNextId()) {}
virtual int position() const {
UNREACHABLE();
@@ -269,10 +269,12 @@
}
unsigned id() const { return id_; }
+ unsigned test_id() const { return test_id_; }
private:
ExternalArrayType external_array_type_;
unsigned id_;
+ unsigned test_id_;
};
Index: src/full-codegen.cc
===================================================================
--- src/full-codegen.cc (revision 8228)
+++ src/full-codegen.cc (working copy)
@@ -441,7 +441,7 @@
// For simplicity we always test the accumulator register.
__ Move(result_register(), reg);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -463,7 +463,7 @@
// For simplicity we always test the accumulator register.
__ pop(result_register());
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -744,9 +744,9 @@
Label discard, restore;
PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
if (is_logical_and) {
- DoTest(&discard, &restore, &restore);
+ DoTest(expr, &discard, &restore, &restore);
} else {
- DoTest(&restore, &discard, &restore);
+ DoTest(expr, &restore, &discard, &restore);
}
__ bind(&restore);
__ pop(result_register());
@@ -763,9 +763,9 @@
Label discard;
PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
if (is_logical_and) {
- DoTest(&discard, &done, &discard);
+ DoTest(expr, &discard, &done, &discard);
} else {
- DoTest(&done, &discard, &discard);
+ DoTest(expr, &done, &discard, &discard);
}
__ bind(&discard);
__ Drop(1);
Index: src/full-codegen.h
===================================================================
--- src/full-codegen.h (revision 8228)
+++ src/full-codegen.h (working copy)
@@ -298,7 +298,10 @@
// Helper function to convert a pure value into a test context. The
value
// is expected on the stack or the accumulator, depending on the
platform.
// See the platform-specific implementation for details.
- void DoTest(Label* if_true, Label* if_false, Label* fall_through);
+ void DoTest(Expression* condition,
+ Label* if_true,
+ Label* if_false,
+ Label* fall_through);
// Helper function to split control flow and avoid a branch to the
// fall-through label if it is set up.
@@ -347,7 +350,7 @@
Label* if_true,
Label* if_false,
Label* fall_through) {
- TestContext context(this, if_true, if_false, fall_through);
+ TestContext context(this, expr, if_true, if_false, fall_through);
VisitInCurrentContext(expr);
}
@@ -670,11 +673,13 @@
class TestContext : public ExpressionContext {
public:
- explicit TestContext(FullCodeGenerator* codegen,
- Label* true_label,
- Label* false_label,
- Label* fall_through)
+ TestContext(FullCodeGenerator* codegen,
+ Expression* condition,
+ Label* true_label,
+ Label* false_label,
+ Label* fall_through)
: ExpressionContext(codegen),
+ condition_(condition),
true_label_(true_label),
false_label_(false_label),
fall_through_(fall_through) { }
@@ -704,6 +709,7 @@
virtual bool IsTest() const { return true; }
private:
+ Expression* condition_;
Label* true_label_;
Label* false_label_;
Label* fall_through_;
Index: src/hydrogen.cc
===================================================================
--- src/hydrogen.cc (revision 8228)
+++ src/hydrogen.cc (working copy)
@@ -1985,9 +1985,10 @@
HBasicBlock* if_false = owner->graph()->CreateBasicBlock();
if_true->MarkAsInlineReturnTarget();
if_false->MarkAsInlineReturnTarget();
+ Expression* cond =
TestContext::cast(owner->ast_context())->condition();
// The AstContext constructor pushed on the context stack. This
newed
// instance is the reason that AstContext can't be BASE_EMBEDDED.
- test_context_ = new TestContext(owner, if_true, if_false);
+ test_context_ = new TestContext(owner, cond, if_true, if_false);
} else {
function_return_ = owner->graph()->CreateBasicBlock();
function_return()->MarkAsInlineReturnTarget();
@@ -2157,7 +2158,7 @@
void HGraphBuilder::VisitForControl(Expression* expr,
HBasicBlock* true_block,
HBasicBlock* false_block) {
- TestContext for_test(this, true_block, false_block);
+ TestContext for_test(this, expr, true_block, false_block);
Visit(expr);
}
Index: src/hydrogen.h
===================================================================
--- src/hydrogen.h (revision 8228)
+++ src/hydrogen.h (working copy)
@@ -543,9 +543,11 @@
class TestContext: public AstContext {
public:
TestContext(HGraphBuilder* owner,
+ Expression* condition,
HBasicBlock* if_true,
HBasicBlock* if_false)
: AstContext(owner, Expression::kTest),
+ condition_(condition),
if_true_(if_true),
if_false_(if_false) {
}
@@ -558,6 +560,7 @@
return reinterpret_cast<TestContext*>(context);
}
+ Expression* condition() const { return condition_; }
HBasicBlock* if_true() const { return if_true_; }
HBasicBlock* if_false() const { return if_false_; }
@@ -566,6 +569,7 @@
// control flow.
void BuildBranch(HValue* value);
+ Expression* condition_;
HBasicBlock* if_true_;
HBasicBlock* if_false_;
};
Index: src/ia32/full-codegen-ia32.cc
===================================================================
--- src/ia32/full-codegen-ia32.cc (revision 8228)
+++ src/ia32/full-codegen-ia32.cc (working copy)
@@ -374,7 +374,7 @@
// For simplicity we always test the accumulator register.
codegen()->Move(result_register(), slot);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -448,7 +448,7 @@
} else {
// For simplicity we always test the accumulator register.
__ mov(result_register(), lit);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_,
fall_through_);
}
}
@@ -484,7 +484,7 @@
__ Drop(count);
__ Move(result_register(), reg);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -561,7 +561,8 @@
}
-void FullCodeGenerator::DoTest(Label* if_true,
+void FullCodeGenerator::DoTest(Expression* condition,
+ Label* if_true,
Label* if_false,
Label* fall_through) {
ToBooleanStub stub;
Index: src/x64/full-codegen-x64.cc
===================================================================
--- src/x64/full-codegen-x64.cc (revision 8228)
+++ src/x64/full-codegen-x64.cc (working copy)
@@ -377,7 +377,7 @@
void FullCodeGenerator::TestContext::Plug(Slot* slot) const {
codegen()->Move(result_register(), slot);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -410,7 +410,7 @@
if (true_label_ != fall_through_) __ jmp(true_label_);
} else {
__ LoadRoot(result_register(), index);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_,
fall_through_);
}
}
@@ -455,7 +455,7 @@
} else {
// For simplicity we always test the accumulator register.
__ Move(result_register(), lit);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_,
fall_through_);
}
}
@@ -491,7 +491,7 @@
__ Drop(count);
__ Move(result_register(), reg);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
- codegen()->DoTest(true_label_, false_label_, fall_through_);
+ codegen()->DoTest(condition_, true_label_, false_label_, fall_through_);
}
@@ -566,7 +566,8 @@
}
-void FullCodeGenerator::DoTest(Label* if_true,
+void FullCodeGenerator::DoTest(Expression* condition,
+ Label* if_true,
Label* if_false,
Label* fall_through) {
ToBooleanStub stub;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
