Commit 203438d9 authored by bmeurer's avatar bmeurer Committed by Commit bot

[turbofan] Connect non-terminating loops via Terminate.

This revives the Terminate operator and removes the weird Always
operator. As a first step we let the ControlReducer connect non
terminating loops via Terminate. The next step will be to change the
graph builder to insert Terminate nodes into every loop.

Review URL: https://codereview.chromium.org/1123213002

Cr-Commit-Position: refs/heads/master@{#28259}
parent 6fb1e76d
......@@ -142,7 +142,6 @@ std::ostream& operator<<(std::ostream& os, ParameterInfo const& i) {
#define CACHED_OP_LIST(V) \
V(Always, Operator::kPure, 0, 0, 0, 1, 0, 0) \
V(Dead, Operator::kFoldable, 0, 0, 0, 0, 0, 1) \
V(End, Operator::kKontrol, 0, 0, 1, 0, 0, 0) \
V(IfTrue, Operator::kKontrol, 0, 0, 1, 0, 0, 1) \
......@@ -153,6 +152,7 @@ std::ostream& operator<<(std::ostream& os, ParameterInfo const& i) {
V(Throw, Operator::kKontrol, 1, 1, 1, 0, 0, 1) \
V(Deoptimize, Operator::kNoThrow, 1, 1, 1, 0, 0, 1) \
V(Return, Operator::kNoThrow, 1, 1, 1, 0, 0, 1) \
V(Terminate, Operator::kNoThrow, 0, 1, 1, 0, 0, 1) \
V(OsrNormalEntry, Operator::kFoldable, 0, 1, 1, 0, 1, 1) \
V(OsrLoopEntry, Operator::kFoldable, 0, 1, 1, 0, 1, 1)
......
......@@ -183,11 +183,6 @@ class CommonOperatorBuilder final : public ZoneObject {
public:
explicit CommonOperatorBuilder(Zone* zone);
// Special operator used only in Branches to mark them as always taken, but
// still unfoldable. This is required to properly connect non terminating
// loops to end (in both the sea of nodes and the CFG).
const Operator* Always();
const Operator* Dead();
const Operator* End();
const Operator* Branch(BranchHint = BranchHint::kNone);
......@@ -201,6 +196,7 @@ class CommonOperatorBuilder final : public ZoneObject {
const Operator* Throw();
const Operator* Deoptimize();
const Operator* Return();
const Operator* Terminate();
const Operator* Start(int num_formal_parameters);
const Operator* Loop(int control_input_count);
......
......@@ -150,17 +150,11 @@ class ControlReducerImpl final : public AdvancedReducer {
TRACE("ConnectNTL: #%d:%s\n", loop->id(), loop->op()->mnemonic());
DCHECK_EQ(IrOpcode::kLoop, loop->opcode());
Node* always = graph()->NewNode(common()->Always());
Node* branch = graph()->NewNode(common()->Branch(), always, loop);
Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
// Insert the branch into the loop and collect all loop effects.
// Collect all loop effects.
NodeVector effects(zone_);
for (auto edge : loop->use_edges()) {
DCHECK_EQ(loop, edge.to());
DCHECK(NodeProperties::IsControlEdge(edge));
if (edge.from() == branch) continue;
switch (edge.from()->opcode()) {
case IrOpcode::kPhi:
break;
......@@ -168,8 +162,6 @@ class ControlReducerImpl final : public AdvancedReducer {
effects.push_back(edge.from());
break;
default:
// Update all control edges (except {branch}) pointing to the {loop}.
edge.UpdateTo(if_true);
break;
}
}
......@@ -184,33 +176,32 @@ class ControlReducerImpl final : public AdvancedReducer {
effects_count, &effects.front());
}
// Add a return to connect the NTL to the end.
Node* ret = graph()->NewNode(
common()->Return(), jsgraph_->UndefinedConstant(), effect, if_false);
// Add a terminate to connect the NTL to the end.
Node* terminate = graph()->NewNode(common()->Terminate(), effect, loop);
Node* end = graph()->end();
if (end->opcode() == IrOpcode::kDead) {
// End is actually the dead node. Make a new end.
end = graph()->NewNode(common()->End(), ret);
end = graph()->NewNode(common()->End(), terminate);
graph()->SetEnd(end);
return end;
}
// End is not dead.
Node* merge = end->InputAt(0);
if (merge == NULL || merge->opcode() == IrOpcode::kDead) {
// The end node died; just connect end to {ret}.
end->ReplaceInput(0, ret);
// The end node died; just connect end to {terminate}.
end->ReplaceInput(0, terminate);
} else if (merge->opcode() != IrOpcode::kMerge) {
// Introduce a final merge node for {end->InputAt(0)} and {ret}.
merge = graph()->NewNode(common()->Merge(2), merge, ret);
// Introduce a final merge node for {end->InputAt(0)} and {terminate}.
merge = graph()->NewNode(common()->Merge(2), merge, terminate);
end->ReplaceInput(0, merge);
ret = merge;
terminate = merge;
} else {
// Append a new input to the final merge at the end.
merge->AppendInput(graph()->zone(), ret);
merge->AppendInput(graph()->zone(), terminate);
merge->set_op(common()->Merge(merge->InputCount()));
}
return ret;
return terminate;
}
void AddNodesReachableFromRoots(ReachabilityMarker& marked,
......
......@@ -474,9 +474,6 @@ void InstructionSelector::VisitControl(BasicBlock* block) {
BasicBlock* tbranch = block->SuccessorAt(0);
BasicBlock* fbranch = block->SuccessorAt(1);
if (tbranch == fbranch) return VisitGoto(tbranch);
// Treat special Branch(Always, IfTrue, IfFalse) as Goto(IfTrue).
Node* const condition = input->InputAt(0);
if (condition->opcode() == IrOpcode::kAlways) return VisitGoto(tbranch);
return VisitBranch(input, tbranch, fbranch);
}
case BasicBlock::kSwitch: {
......@@ -550,6 +547,7 @@ void InstructionSelector::VisitNode(Node* node) {
case IrOpcode::kIfDefault:
case IrOpcode::kEffectPhi:
case IrOpcode::kMerge:
case IrOpcode::kTerminate:
// No code needed for these graph artifacts.
return;
case IrOpcode::kIfException:
......
......@@ -38,11 +38,9 @@ Reduction JSGenericLowering::Reduce(Node* node) {
// poor-man's representation inference here and insert manual change.
if (!is_typing_enabled_) {
Node* condition = node->InputAt(0);
if (condition->opcode() != IrOpcode::kAlways) {
Node* test = graph()->NewNode(machine()->WordEqual(), condition,
jsgraph()->TrueConstant());
node->ReplaceInput(0, test);
}
Node* test = graph()->NewNode(machine()->WordEqual(), condition,
jsgraph()->TrueConstant());
node->ReplaceInput(0, test);
break;
}
// Fall-through.
......
......@@ -22,6 +22,7 @@
V(Deoptimize) \
V(Return) \
V(TailCall) \
V(Terminate) \
V(OsrNormalEntry) \
V(OsrLoopEntry) \
V(Throw) \
......@@ -54,8 +55,7 @@
#define COMMON_OP_LIST(V) \
CONSTANT_OP_LIST(V) \
INNER_OP_LIST(V) \
V(Always)
INNER_OP_LIST(V)
// Opcodes for JavaScript operators.
#define JS_COMPARE_BINOP_LIST(V) \
......@@ -310,7 +310,7 @@ class IrOpcode {
// Returns true if opcode for common operator.
static bool IsCommonOpcode(Value value) {
return kStart <= value && value <= kAlways;
return kStart <= value && value <= kProjection;
}
// Returns true if opcode for control operator.
......
......@@ -313,6 +313,13 @@ class CFGBuilder : public ZoneObject {
case IrOpcode::kMerge:
BuildBlockForNode(node);
break;
case IrOpcode::kTerminate: {
// Put Terminate in the loop to which it refers.
Node* loop = NodeProperties::GetControlInput(node);
BasicBlock* block = BuildBlockForNode(loop);
FixNode(block, node);
break;
}
case IrOpcode::kBranch:
case IrOpcode::kSwitch:
BuildBlocksForSuccessors(node);
......
......@@ -500,8 +500,6 @@ class RepresentationSelector {
SetOutput(node, kRepTagged | changer_->TypeFromUpperBound(upper));
return;
}
case IrOpcode::kAlways:
return VisitLeaf(node, kRepBit);
case IrOpcode::kInt32Constant:
return VisitLeaf(node, kRepWord32);
case IrOpcode::kInt64Constant:
......
......@@ -253,6 +253,7 @@ class Typer::Visitor : public Reducer {
DECLARE_CASE(Deoptimize)
DECLARE_CASE(Return)
DECLARE_CASE(TailCall)
DECLARE_CASE(Terminate)
DECLARE_CASE(OsrNormalEntry)
DECLARE_CASE(OsrLoopEntry)
DECLARE_CASE(Throw)
......@@ -297,6 +298,7 @@ class Typer::Visitor : public Reducer {
DECLARE_CASE(Deoptimize)
DECLARE_CASE(Return)
DECLARE_CASE(TailCall)
DECLARE_CASE(Terminate)
DECLARE_CASE(OsrNormalEntry)
DECLARE_CASE(OsrLoopEntry)
DECLARE_CASE(Throw)
......@@ -643,11 +645,6 @@ Bounds Typer::Visitor::TypeIfException(Node* node) {
// Common operators.
Bounds Typer::Visitor::TypeAlways(Node* node) {
return Bounds(Type::None(zone()), Type::Boolean(zone()));
}
Bounds Typer::Visitor::TypeParameter(Node* node) {
return Bounds::Unbounded(zone());
}
......
......@@ -168,16 +168,6 @@ void Verifier::Visitor::Check(Node* node) {
}
switch (node->opcode()) {
case IrOpcode::kAlways:
// Always has no inputs.
CHECK_EQ(0, input_count);
// Always uses are Branch.
for (auto use : node->uses()) {
CHECK(use->opcode() == IrOpcode::kBranch);
}
// Type is boolean.
CheckUpperIs(node, Type::Boolean());
break;
case IrOpcode::kStart:
// Start has no inputs.
CHECK_EQ(0, input_count);
......@@ -293,6 +283,15 @@ void Verifier::Visitor::Check(Node* node) {
// Type is empty.
CheckNotTyped(node);
break;
case IrOpcode::kTerminate:
CHECK_EQ(IrOpcode::kLoop,
NodeProperties::GetControlInput(node)->opcode());
// Type is empty.
CheckNotTyped(node);
CHECK_EQ(1, control_count);
CHECK_EQ(1, effect_count);
CHECK_EQ(2, input_count);
break;
case IrOpcode::kOsrNormalEntry:
case IrOpcode::kOsrLoopEntry:
// Osr entries have
......
......@@ -48,7 +48,6 @@ const SharedOperator kSharedOperators[] = {
value_input_count, effect_input_count, control_input_count, \
value_output_count, effect_output_count, control_output_count \
}
SHARED(Always, Operator::kPure, 0, 0, 0, 1, 0, 0),
SHARED(Dead, Operator::kFoldable, 0, 0, 0, 0, 0, 1),
SHARED(End, Operator::kKontrol, 0, 0, 1, 0, 0, 0),
SHARED(IfTrue, Operator::kKontrol, 0, 0, 1, 0, 0, 1),
......@@ -56,7 +55,8 @@ const SharedOperator kSharedOperators[] = {
SHARED(IfSuccess, Operator::kKontrol, 0, 0, 1, 0, 0, 1),
SHARED(IfException, Operator::kKontrol, 0, 0, 1, 1, 0, 1),
SHARED(Throw, Operator::kKontrol, 1, 1, 1, 0, 0, 1),
SHARED(Return, Operator::kNoThrow, 1, 1, 1, 0, 0, 1)
SHARED(Return, Operator::kNoThrow, 1, 1, 1, 0, 0, 1),
SHARED(Terminate, Operator::kNoThrow, 0, 1, 1, 0, 0, 1)
#undef SHARED
};
......
......@@ -57,18 +57,11 @@ TEST_F(ControlReducerTest, NonTerminatingLoop) {
Node* loop = graph()->NewNode(common()->Loop(2), graph()->start());
loop->AppendInput(graph()->zone(), loop);
ReduceGraph();
Capture<Node*> branch;
EXPECT_THAT(
graph()->end(),
IsEnd(IsMerge(
graph()->start(),
IsReturn(IsUndefinedConstant(), graph()->start(),
IsIfFalse(
AllOf(CaptureEq(&branch),
IsBranch(IsAlways(),
AllOf(loop, IsLoop(graph()->start(),
IsIfTrue(CaptureEq(
&branch)))))))))));
IsEnd(IsMerge(graph()->start(),
IsTerminate(graph()->start(),
AllOf(loop, IsLoop(graph()->start(), loop))))));
}
......@@ -79,19 +72,12 @@ TEST_F(ControlReducerTest, NonTerminatingLoopWithEffectPhi) {
ephi->AppendInput(graph()->zone(), ephi);
ephi->AppendInput(graph()->zone(), loop);
ReduceGraph();
Capture<Node*> branch;
EXPECT_THAT(
graph()->end(),
IsEnd(IsMerge(
graph()->start(),
IsReturn(IsUndefinedConstant(),
AllOf(ephi, IsEffectPhi(graph()->start(), ephi, loop)),
IsIfFalse(
AllOf(CaptureEq(&branch),
IsBranch(IsAlways(),
AllOf(loop, IsLoop(graph()->start(),
IsIfTrue(CaptureEq(
&branch)))))))))));
IsTerminate(AllOf(ephi, IsEffectPhi(graph()->start(), ephi, loop)),
AllOf(loop, IsLoop(graph()->start(), loop))))));
}
......@@ -105,22 +91,15 @@ TEST_F(ControlReducerTest, NonTerminatingLoopWithTwoEffectPhis) {
ephi2->AppendInput(graph()->zone(), ephi2);
ephi2->AppendInput(graph()->zone(), loop);
ReduceGraph();
Capture<Node*> branch;
EXPECT_THAT(
graph()->end(),
IsEnd(IsMerge(
graph()->start(),
IsReturn(
IsUndefinedConstant(),
IsTerminate(
IsEffectSet(
AllOf(ephi1, IsEffectPhi(graph()->start(), ephi1, loop)),
AllOf(ephi2, IsEffectPhi(graph()->start(), ephi2, loop))),
IsIfFalse(AllOf(
CaptureEq(&branch),
IsBranch(
IsAlways(),
AllOf(loop, IsLoop(graph()->start(),
IsIfTrue(CaptureEq(&branch)))))))))));
AllOf(loop, IsLoop(graph()->start(), loop))))));
}
......@@ -129,16 +108,9 @@ TEST_F(ControlReducerTest, NonTerminatingLoopWithDeadEnd) {
loop->AppendInput(graph()->zone(), loop);
graph()->end()->ReplaceInput(0, graph()->NewNode(common()->Dead()));
ReduceGraph();
Capture<Node*> branch;
EXPECT_THAT(
graph()->end(),
IsEnd(IsReturn(
IsUndefinedConstant(), graph()->start(),
IsIfFalse(AllOf(
CaptureEq(&branch),
IsBranch(IsAlways(),
AllOf(loop, IsLoop(graph()->start(),
IsIfTrue(CaptureEq(&branch))))))))));
EXPECT_THAT(graph()->end(),
IsEnd(IsTerminate(graph()->start(),
AllOf(loop, IsLoop(graph()->start(), loop)))));
}
......
......@@ -323,6 +323,37 @@ class IsReturnMatcher final : public NodeMatcher {
};
class IsTerminateMatcher final : public NodeMatcher {
public:
IsTerminateMatcher(const Matcher<Node*>& effect_matcher,
const Matcher<Node*>& control_matcher)
: NodeMatcher(IrOpcode::kTerminate),
effect_matcher_(effect_matcher),
control_matcher_(control_matcher) {}
void DescribeTo(std::ostream* os) const final {
NodeMatcher::DescribeTo(os);
*os << " whose effect (";
effect_matcher_.DescribeTo(os);
*os << ") and control (";
control_matcher_.DescribeTo(os);
*os << ")";
}
bool MatchAndExplain(Node* node, MatchResultListener* listener) const final {
return (NodeMatcher::MatchAndExplain(node, listener) &&
PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect",
effect_matcher_, listener) &&
PrintMatchAndExplain(NodeProperties::GetControlInput(node),
"control", control_matcher_, listener));
}
private:
const Matcher<Node*> effect_matcher_;
const Matcher<Node*> control_matcher_;
};
template <typename T>
class IsConstantMatcher final : public NodeMatcher {
public:
......@@ -1289,11 +1320,6 @@ class IsUnopMatcher final : public NodeMatcher {
} // namespace
Matcher<Node*> IsAlways() {
return MakeMatcher(new NodeMatcher(IrOpcode::kAlways));
}
Matcher<Node*> IsEnd(const Matcher<Node*>& control_matcher) {
return MakeMatcher(new IsControl1Matcher(IrOpcode::kEnd, control_matcher));
}
......@@ -1389,6 +1415,12 @@ Matcher<Node*> IsReturn(const Matcher<Node*>& value_matcher,
}
Matcher<Node*> IsTerminate(const Matcher<Node*>& effect_matcher,
const Matcher<Node*>& control_matcher) {
return MakeMatcher(new IsTerminateMatcher(effect_matcher, control_matcher));
}
Matcher<Node*> IsExternalConstant(
const Matcher<ExternalReference>& value_matcher) {
return MakeMatcher(new IsConstantMatcher<ExternalReference>(
......
......@@ -31,7 +31,6 @@ class Node;
using ::testing::Matcher;
Matcher<Node*> IsAlways();
Matcher<Node*> IsEnd(const Matcher<Node*>& control_matcher);
Matcher<Node*> IsBranch(const Matcher<Node*>& value_matcher,
const Matcher<Node*>& control_matcher);
......@@ -59,6 +58,8 @@ Matcher<Node*> IsFinish(const Matcher<Node*>& value_matcher,
Matcher<Node*> IsReturn(const Matcher<Node*>& value_matcher,
const Matcher<Node*>& effect_matcher,
const Matcher<Node*>& control_matcher);
Matcher<Node*> IsTerminate(const Matcher<Node*>& effect_matcher,
const Matcher<Node*>& control_matcher);
Matcher<Node*> IsExternalConstant(
const Matcher<ExternalReference>& value_matcher);
Matcher<Node*> IsHeapConstant(
......
......@@ -2467,6 +2467,29 @@ TARGET_TEST_F(SchedulerTest, FloatingSwitch) {
ComputeAndVerifySchedule(16);
}
TARGET_TEST_F(SchedulerTest, Terminate) {
Node* start = graph()->NewNode(common()->Start(1));
graph()->SetStart(start);
Node* loop = graph()->NewNode(common()->Loop(2), start, start);
loop->ReplaceInput(1, loop); // self loop, NTL.
Node* effect = graph()->NewNode(common()->EffectPhi(1), start, loop);
Node* terminate = graph()->NewNode(common()->Terminate(), effect, loop);
effect->ReplaceInput(1, terminate);
Node* end = graph()->NewNode(common()->End(), terminate);
graph()->SetEnd(end);
Schedule* schedule = ComputeAndVerifySchedule(6);
BasicBlock* block = schedule->block(loop);
EXPECT_EQ(block, schedule->block(effect));
EXPECT_GE(block->rpo_number(), 0);
}
} // namespace compiler
} // namespace internal
} // namespace v8
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