diff options
author | Chandler Carruth <chandlerc@gmail.com> | 2017-08-08 10:13:23 +0000 |
---|---|---|
committer | Chandler Carruth <chandlerc@gmail.com> | 2017-08-08 10:13:23 +0000 |
commit | 6e35c31d2d2c1853113ca0526ceead85e9c8e1ac (patch) | |
tree | 52e2252a93d643471106d3f9a869e9c10339fad4 /llvm/lib/Analysis/CGSCCPassManager.cpp | |
parent | 95bd95c075067d904eaaad9c29a95afd16676952 (diff) | |
download | bcm5719-llvm-6e35c31d2d2c1853113ca0526ceead85e9c8e1ac.tar.gz bcm5719-llvm-6e35c31d2d2c1853113ca0526ceead85e9c8e1ac.zip |
[PM] Fix a likely more critical infloop bug in the CGSCC pass manager.
This was just a bad oversight on my part. The code in question should
never have worked without this fix. But it turns out, there are
relatively few places that involve libfunctions that participate in
a single SCC, and unless they do, this happens to not matter.
The effect of not having this correct is that each time through this
routine, the edge from write_wrapper to write was toggled between a call
edge and a ref edge. First time through, it becomes a demoted call edge
and is turned into a ref edge. Next time it is a promoted call edge from
a ref edge. On, and on it goes forever.
I've added the asserts which should have always been here to catch silly
mistakes like this in the future as well as a test case that will
actually infloop without the fix.
The other (much scarier) infinite-inlining issue I think didn't actually
occur in practice, and I simply misdiagnosed this minor issue as that
much more scary issue. The other issue *is* still a real issue, but I'm
somewhat relieved that so far it hasn't happened in real-world code
yet...
llvm-svn: 310342
Diffstat (limited to 'llvm/lib/Analysis/CGSCCPassManager.cpp')
-rw-r--r-- | llvm/lib/Analysis/CGSCCPassManager.cpp | 13 |
1 files changed, 10 insertions, 3 deletions
diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp index cb6d139c88c..b81069559c7 100644 --- a/llvm/lib/Analysis/CGSCCPassManager.cpp +++ b/llvm/lib/Analysis/CGSCCPassManager.cpp @@ -421,7 +421,9 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( assert(E && "No function transformations should introduce *new* " "call edges! Any new calls should be modeled as " "promoted existing ref edges!"); - RetainedEdges.insert(&CalleeN); + bool Inserted = RetainedEdges.insert(&CalleeN).second; + (void)Inserted; + assert(Inserted && "We should never visit a function twice."); if (!E->isCall()) PromotedRefTargets.insert(&CalleeN); } @@ -441,7 +443,9 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( assert(E && "No function transformations should introduce *new* ref " "edges! Any new ref edges would require IPO which " "function passes aren't allowed to do!"); - RetainedEdges.insert(&RefereeN); + bool Inserted = RetainedEdges.insert(&RefereeN).second; + (void)Inserted; + assert(Inserted && "We should never visit a function twice."); if (E->isCall()) DemotedCallTargets.insert(&RefereeN); }; @@ -449,7 +453,10 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( // Include synthetic reference edges to known, defined lib functions. for (auto *F : G.getLibFunctions()) - VisitRef(*F); + // While the list of lib functions doesn't have repeats, don't re-visit + // anything handled above. + if (!Visited.count(F)) + VisitRef(*F); // First remove all of the edges that are no longer present in this function. // We have to build a list of dead targets first and then remove them as the |