Commit 69d3f9c6 by Jim Stichnoth

Subzero: Prune unreachable nodes after constructing the Cfg.

The gcc torture test suite has examples where there is a function call (to a routine that throws an exception or aborts or something), followed by an "unreachable" instruction, followed by more code that may e.g. return a value to the caller. In these examples, the code following the unreachable is itself unreachable. Problems arise when the unreachable code references a variable defined in the reachable code. This triggers a liveness consistency error because the use of the variable has no reaching definition. It's a bit surprising that LLVM actually allows this, but it does so we need to deal with it. The solution is, after initial CFG construction, do a traversal starting from the entry node and then delete any undiscovered nodes. There is code in Subzero that assumes Cfg::Nodes[i]->Number == i, so the nodes need to be renumbered after pruning. The alternative was to set Nodes[i]=nullptr and not change the node number, but that would mean peppering the code base with CfgNode null checks. BUG= none R=jvoung@chromium.org Review URL: https://codereview.chromium.org/1027933002
parent 27c56bf6
...@@ -108,9 +108,41 @@ void Cfg::translate() { ...@@ -108,9 +108,41 @@ void Cfg::translate() {
getContext()->dumpTimers(); getContext()->dumpTimers();
} }
void Cfg::computePredecessors() { void Cfg::computeInOutEdges() {
// Compute the out-edges.
for (CfgNode *Node : Nodes) for (CfgNode *Node : Nodes)
Node->computePredecessors(); Node->computeSuccessors();
// Prune any unreachable nodes before computing in-edges.
SizeT NumNodes = getNumNodes();
llvm::BitVector Reachable(NumNodes);
llvm::BitVector Pending(NumNodes);
Pending.set(getEntryNode()->getIndex());
while (true) {
int Index = Pending.find_first();
if (Index == -1)
break;
Pending.reset(Index);
Reachable.set(Index);
CfgNode *Node = Nodes[Index];
assert(Node->getIndex() == (SizeT)Index);
for (CfgNode *Succ : Node->getOutEdges()) {
SizeT SuccIndex = Succ->getIndex();
if (!Reachable.test(SuccIndex))
Pending.set(SuccIndex);
}
}
SizeT Dest = 0;
for (SizeT Source = 0; Source < NumNodes; ++Source) {
if (Reachable.test(Source)) {
Nodes[Dest] = Nodes[Source];
Nodes[Dest]->resetIndex(Dest);
// Compute the in-edges.
Nodes[Dest]->computePredecessors();
++Dest;
}
}
Nodes.resize(Dest);
} }
void Cfg::renumberInstructions() { void Cfg::renumberInstructions() {
......
...@@ -135,9 +135,9 @@ public: ...@@ -135,9 +135,9 @@ public:
// Passes over the CFG. // Passes over the CFG.
void translate(); void translate();
// After the CFG is fully constructed, iterate over the nodes and // After the CFG is fully constructed, iterate over the nodes and
// compute the predecessor edges, in the form of // compute the predecessor and successor edges, in the form of
// CfgNode::InEdges[]. // CfgNode::InEdges[] and CfgNode::OutEdges[].
void computePredecessors(); void computeInOutEdges();
void renumberInstructions(); void renumberInstructions();
void placePhiLoads(); void placePhiLoads();
void placePhiStores(); void placePhiStores();
......
...@@ -69,11 +69,14 @@ void CfgNode::renumberInstructions() { ...@@ -69,11 +69,14 @@ void CfgNode::renumberInstructions() {
// constructed, the computePredecessors() pass finalizes it by // constructed, the computePredecessors() pass finalizes it by
// creating the InEdges list. // creating the InEdges list.
void CfgNode::computePredecessors() { void CfgNode::computePredecessors() {
OutEdges = Insts.rbegin()->getTerminatorEdges();
for (CfgNode *Succ : OutEdges) for (CfgNode *Succ : OutEdges)
Succ->InEdges.push_back(this); Succ->InEdges.push_back(this);
} }
void CfgNode::computeSuccessors() {
OutEdges = Insts.rbegin()->getTerminatorEdges();
}
// This does part 1 of Phi lowering, by creating a new dest variable // This does part 1 of Phi lowering, by creating a new dest variable
// for each Phi instruction, replacing the Phi instruction's dest with // for each Phi instruction, replacing the Phi instruction's dest with
// that variable, and adding an explicit assignment of the old dest to // that variable, and adding an explicit assignment of the old dest to
...@@ -605,19 +608,21 @@ bool CfgNode::liveness(Liveness *Liveness) { ...@@ -605,19 +608,21 @@ bool CfgNode::liveness(Liveness *Liveness) {
// entry. // entry.
bool IsEntry = (Func->getEntryNode() == this); bool IsEntry = (Func->getEntryNode() == this);
if (!(IsEntry || Live == LiveOrig)) { if (!(IsEntry || Live == LiveOrig)) {
// This is a fatal liveness consistency error. Print some if (ALLOW_DUMP) {
// diagnostics and abort. // This is a fatal liveness consistency error. Print some
Ostream &Str = Func->getContext()->getStrDump(); // diagnostics and abort.
Func->resetCurrentNode(); Ostream &Str = Func->getContext()->getStrDump();
Str << "LiveOrig-Live ="; Func->resetCurrentNode();
for (SizeT i = Live.size(); i < LiveOrig.size(); ++i) { Str << "LiveOrig-Live =";
if (LiveOrig.test(i)) { for (SizeT i = Live.size(); i < LiveOrig.size(); ++i) {
Str << " "; if (LiveOrig.test(i)) {
Liveness->getVariable(i, this)->dump(Func); Str << " ";
Liveness->getVariable(i, this)->dump(Func);
}
} }
Str << "\n";
} }
Str << "\n"; llvm::report_fatal_error("Fatal inconsistency in liveness analysis");
llvm_unreachable("Fatal inconsistency in liveness analysis");
} }
bool Changed = false; bool Changed = false;
......
...@@ -33,6 +33,7 @@ public: ...@@ -33,6 +33,7 @@ public:
// Access the label number and name for this node. // Access the label number and name for this node.
SizeT getIndex() const { return Number; } SizeT getIndex() const { return Number; }
void resetIndex(SizeT NewNumber) { Number = NewNumber; }
IceString getName() const; IceString getName() const;
void setName(const IceString &NewName) { void setName(const IceString &NewName) {
// Make sure that the name can only be set once. // Make sure that the name can only be set once.
...@@ -70,6 +71,7 @@ public: ...@@ -70,6 +71,7 @@ public:
// Add a predecessor edge to the InEdges list for each of this // Add a predecessor edge to the InEdges list for each of this
// node's successors. // node's successors.
void computePredecessors(); void computePredecessors();
void computeSuccessors();
CfgNode *splitIncomingEdge(CfgNode *Pred, SizeT InEdgeIndex); CfgNode *splitIncomingEdge(CfgNode *Pred, SizeT InEdgeIndex);
void placePhiLoads(); void placePhiLoads();
...@@ -92,7 +94,7 @@ public: ...@@ -92,7 +94,7 @@ public:
private: private:
CfgNode(Cfg *Func, SizeT LabelIndex); CfgNode(Cfg *Func, SizeT LabelIndex);
Cfg *const Func; Cfg *const Func;
const SizeT Number; // label index SizeT Number; // label index
Cfg::IdentifierIndexType NameIndex; // index into Cfg::NodeNames table Cfg::IdentifierIndexType NameIndex; // index into Cfg::NodeNames table
bool HasReturn; // does this block need an epilog? bool HasReturn; // does this block need an epilog?
bool NeedsPlacement; bool NeedsPlacement;
......
...@@ -113,7 +113,7 @@ public: ...@@ -113,7 +113,7 @@ public:
for (const BasicBlock &BBI : *F) for (const BasicBlock &BBI : *F)
convertBasicBlock(&BBI); convertBasicBlock(&BBI);
Func->setEntryNode(mapBasicBlockToNode(&F->getEntryBlock())); Func->setEntryNode(mapBasicBlockToNode(&F->getEntryBlock()));
Func->computePredecessors(); Func->computeInOutEdges();
Ice::Cfg::setCurrentCfg(nullptr); Ice::Cfg::setCurrentCfg(nullptr);
Converter.translateFcn(std::move(Func)); Converter.translateFcn(std::move(Func));
......
...@@ -1920,7 +1920,7 @@ void FunctionParser::ExitBlock() { ...@@ -1920,7 +1920,7 @@ void FunctionParser::ExitBlock() {
} }
++Index; ++Index;
} }
Func->computePredecessors(); Func->computeInOutEdges();
} }
void FunctionParser::ReportInvalidBinaryOp(Ice::InstArithmetic::OpKind Op, void FunctionParser::ReportInvalidBinaryOp(Ice::InstArithmetic::OpKind Op,
......
; This tests that unreachable basic blocks are pruned from the CFG, so that
; liveness analysis doesn't detect inconsistencies.
; RUN: %p2i -i %s --filetype=obj --disassemble --args -Om1 | FileCheck %s
; RUN: %p2i -i %s --filetype=obj --disassemble --args -O2 | FileCheck %s
declare void @abort()
define i32 @unreachable_block() {
entry:
; ret_val has no reaching uses and so its assignment may be
; dead-code eliminated.
%ret_val = add i32 undef, undef
call void @abort()
unreachable
label:
; ret_val has no reaching definitions, causing an inconsistency in
; liveness analysis.
ret i32 %ret_val
}
; CHECK-LABEL: unreachable_block
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