Reviewers: jarin,
Description:
[turbofan] Perform OSR deconstruction early and remove type propagation.
This way we don't have to deal with dead pre-OSR code in the graph and
risk optimizing the wrong code, especially we don't make optimistic
assumptions in the dead code that leaks into the OSR code (i.e. deopt
guards are in dead code, but the types propagate to OSR code via the
OsrValue type back propagation).
BUG=v8:4273
LOG=n
[email protected]
Please review this at https://codereview.chromium.org/1215333005/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+6, -82 lines):
M src/compiler/osr.cc
M src/compiler/pipeline.cc
M src/compiler/typer.cc
M test/cctest/compiler/test-osr.cc
Index: src/compiler/osr.cc
diff --git a/src/compiler/osr.cc b/src/compiler/osr.cc
index
fb5716a4961052f1857952150a8bd9398ed28aa5..6c4c49de7981657e86ef4c253668ea4cc16a6b7c
100644
--- a/src/compiler/osr.cc
+++ b/src/compiler/osr.cc
@@ -249,40 +249,6 @@ static void PeelOuterLoopsForOsr(Graph* graph,
CommonOperatorBuilder* common,
}
-static void TransferOsrValueTypesFromLoopPhis(Zone* zone, Node*
osr_loop_entry,
- Node* osr_loop) {
- // Find the index of the osr loop entry into the loop.
- int index = 0;
- for (index = 0; index < osr_loop->InputCount(); index++) {
- if (osr_loop->InputAt(index) == osr_loop_entry) break;
- }
- if (index == osr_loop->InputCount()) return;
-
- for (Node* osr_value : osr_loop_entry->uses()) {
- if (osr_value->opcode() != IrOpcode::kOsrValue) continue;
- bool unknown = true;
- for (Node* phi : osr_value->uses()) {
- if (phi->opcode() != IrOpcode::kPhi) continue;
- if (NodeProperties::GetControlInput(phi) != osr_loop) continue;
- if (phi->InputAt(index) != osr_value) continue;
- if (NodeProperties::IsTyped(phi)) {
- // Transfer the type from the phi to the OSR value itself.
- Bounds phi_bounds = NodeProperties::GetBounds(phi);
- if (unknown) {
- NodeProperties::SetBounds(osr_value, phi_bounds);
- } else {
- Bounds osr_bounds = NodeProperties::GetBounds(osr_value);
- NodeProperties::SetBounds(osr_value,
- Bounds::Both(phi_bounds, osr_bounds,
zone));
- }
- unknown = false;
- }
- }
- if (unknown) NodeProperties::SetBounds(osr_value,
Bounds::Unbounded(zone));
- }
-}
-
-
void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder*
common,
Zone* tmp_zone) {
Graph* graph = jsgraph->graph();
@@ -313,9 +279,6 @@ void OsrHelper::Deconstruct(JSGraph* jsgraph,
CommonOperatorBuilder* common,
CHECK(osr_loop); // Should have found the OSR loop.
- // Transfer the types from loop phis to the OSR values which flow into
them.
- TransferOsrValueTypesFromLoopPhis(graph->zone(), osr_loop_entry,
osr_loop);
-
// Analyze the graph to determine how deeply nested the OSR loop is.
LoopTree* loop_tree = LoopFinder::BuildLoopTree(graph, tmp_zone);
Index: src/compiler/pipeline.cc
diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index
63ba407792875454eca991ccb772c4429f5f3417..66f3bf0fc4a1584d326bd5b4ea8d8ba34bee58f6
100644
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -1041,6 +1041,12 @@ Handle<Code> Pipeline::GenerateCode() {
if (data.compilation_failed()) return Handle<Code>::null();
RunPrintAndVerify("Initial untyped", true);
+ // Perform OSR deconstruction.
+ if (info()->is_osr()) {
+ Run<OsrDeconstructionPhase>();
+ RunPrintAndVerify("OSR deconstruction", true);
+ }
+
// Perform context specialization and inlining (if enabled).
Run<InliningPhase>();
RunPrintAndVerify("Inlined", true);
@@ -1077,11 +1083,6 @@ Handle<Code> Pipeline::GenerateCode() {
RunPrintAndVerify("Loop peeled");
}
- if (info()->is_osr()) {
- Run<OsrDeconstructionPhase>();
- RunPrintAndVerify("OSR deconstruction");
- }
-
if (info()->is_type_feedback_enabled()) {
Run<JSTypeFeedbackPhase>();
RunPrintAndVerify("JSType feedback");
@@ -1101,12 +1102,6 @@ Handle<Code> Pipeline::GenerateCode() {
Run<ChangeLoweringPhase>();
// TODO(jarin, rossberg): Remove UNTYPED once machine typing works.
RunPrintAndVerify("Lowered changes", true);
- } else {
- if (info()->is_osr()) {
- Run<OsrDeconstructionPhase>();
- if (info()->bailout_reason() != kNoReason) return
Handle<Code>::null();
- RunPrintAndVerify("OSR deconstruction", true);
- }
}
// Lower any remaining generic JSOperators.
Index: src/compiler/typer.cc
diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc
index
fa0726c4ac3e6a296ca6b7affbaf94d634b1b8cb..3f4bd27ec6a3ce875d40ee962924a02fe0fc512b
100644
--- a/src/compiler/typer.cc
+++ b/src/compiler/typer.cc
@@ -582,16 +582,6 @@ Bounds Typer::Visitor::TypeParameter(Node* node) {
Bounds Typer::Visitor::TypeOsrValue(Node* node) {
- if (node->InputAt(0)->opcode() == IrOpcode::kOsrLoopEntry) {
- // Before deconstruction, OSR values have type {None} to avoid
polluting
- // the types of phis and other nodes in the graph.
- return Bounds(Type::None(), Type::None());
- }
- if (NodeProperties::IsTyped(node)) {
- // After deconstruction, OSR values may have had a type explicitly set.
- return NodeProperties::GetBounds(node);
- }
- // Otherwise, be conservative.
return Bounds::Unbounded(zone());
}
Index: test/cctest/compiler/test-osr.cc
diff --git a/test/cctest/compiler/test-osr.cc
b/test/cctest/compiler/test-osr.cc
index
77ade9149e3cf6efe963f802d238c40064a99934..80dbccc633f7191759505a8c6e9e97dd19abd1b3
100644
--- a/test/cctest/compiler/test-osr.cc
+++ b/test/cctest/compiler/test-osr.cc
@@ -165,30 +165,6 @@ TEST(Deconstruct_osr1) {
}
-TEST(Deconstruct_osr1_type) {
- OsrDeconstructorTester T(1);
-
- Node* loop = T.NewOsrLoop(1);
- Node* osr_phi =
- T.NewOsrPhi(loop, T.jsgraph.OneConstant(), 0,
T.jsgraph.ZeroConstant());
- Type* type = Type::Signed32();
- NodeProperties::SetBounds(osr_phi, Bounds(type, type));
-
- Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, loop);
- T.graph.SetEnd(ret);
-
- OsrHelper helper(0, 0);
- helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
-
- CHECK_EQ(type, NodeProperties::GetBounds(T.osr_values[0]).lower);
- CHECK_EQ(type, NodeProperties::GetBounds(T.osr_values[0]).upper);
-
- CheckInputs(loop, T.start, loop);
- CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), loop);
- CheckInputs(ret, osr_phi, T.start, loop);
-}
-
-
TEST(Deconstruct_osr_remove_prologue) {
OsrDeconstructorTester T(1);
Diamond d(&T.graph, &T.common, T.p0);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
