[llvm] r336419 - CallGraphSCCPass: iterate over all functions.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 10 09:08:05 PDT 2018
+some debug info cabal folks who might have more detail to help/add/look at
(no doubt Tim can find some of these folks in the Hallowed Halls of his
employer ;) )
On Mon, Jul 9, 2018 at 10:42 PM Mikael Holmén via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Hi,
>
> I wrote
>
> https://bugs.llvm.org/show_bug.cgi?id=38114
>
> about this.
>
> Regards,
> Mikael
>
> On 07/09/2018 10:21 AM, Mikael Holmén via llvm-commits wrote:
> > Hi Tim,
> >
> > I found a case which started going wrong with this patch. I don't know
> > if it's the patch itself that is the problem or if it's just exposing
> > somethng already existing.
> >
> > With
> >
> > opt -S -o - foo.ll -argpromotion -instcombine
> >
> > I get
> >
> > DISubprogram attached to more than one function
> > !1 = distinct !DISubprogram(name: "func_27", scope: !2, file: !2, line:
> > 251, type: !3, isLocal: true, isDefinition: true, scopeLine: 252,
> > isOptimized: false, unit: !5)
> > i64 ()* @func_27
> > LLVM ERROR: Broken module found, compilation aborted!
> >
> > If I add -print-before-all -print-after-all -print-module-scope I get
> > this as the last printouts:
> >
> > *** IR Dump Before Module Verifier *** (function: func_27)
> > ; ModuleID = 'foo.ll'
> > source_filename = "foo.ll"
> >
> > @g_280 = external global i32
> >
> > define i32 @func_1() {
> > bb1:
> > %l_55.115 = alloca i32*, align 8
> > %_tmp555 = load i32*, i32** %l_55.115, align 8
> > %0 = call i64 @func_27()
> > ret i32 undef
> > }
> >
> > define internal i64 @func_27() !dbg !1 {
> > store i32* @g_280, i32** undef, align 8
> > ret i64 undef
> > }
> >
> > define i16 @func_28() {
> > %_tmp5791 = call i32 @func_1()
> > ret i16 undef
> > }
> >
> > !llvm.dbg.cu = !{}
> > !llvm.module.flags = !{!0}
> >
> > !0 = !{i32 2, !"Debug Info Version", i32 3}
> > !1 = distinct !DISubprogram(name: "func_27", scope: !2, file: !2, line:
> > 251, type: !3, isLocal: true, isDefinition: true, scopeLine: 252,
> > isOptimized: false, unit: !5)
> > !2 = !DIFile(filename: "foo.c", directory: "/bar")
> > !3 = !DISubroutineType(types: !4)
> > !4 = !{}
> > !5 = distinct !DICompileUnit(language: DW_LANG_C, file: !2, producer:
> > "My compiler", isOptimized: false, runtimeVersion: 0, emissionKind:
> > LineTablesOnly)
> > DISubprogram attached to more than one function
> > !1 = distinct !DISubprogram(name: "func_27", scope: !2, file: !2, line:
> > 251, type: !3, isLocal: true, isDefinition: true, scopeLine: 252,
> > isOptimized: false, unit: !5)
> > i64 ()* @func_27
> > *** IR Dump After Module Verifier *** (function: func_27)
> > ; ModuleID = 'foo.ll'
> > source_filename = "foo.ll"
> >
> > @g_280 = external global i32
> >
> > define i32 @func_1() {
> > bb1:
> > %l_55.115 = alloca i32*, align 8
> > %_tmp555 = load i32*, i32** %l_55.115, align 8
> > %0 = call i64 @func_27()
> > ret i32 undef
> > }
> >
> > define internal i64 @func_27() !dbg !1 {
> > store i32* @g_280, i32** undef, align 8
> > ret i64 undef
> > }
> >
> > define i16 @func_28() {
> > %_tmp5791 = call i32 @func_1()
> > ret i16 undef
> > }
> >
> > !llvm.dbg.cu = !{}
> > !llvm.module.flags = !{!0}
> >
> > !0 = !{i32 2, !"Debug Info Version", i32 3}
> > !1 = distinct !DISubprogram(name: "func_27", scope: !2, file: !2, line:
> > 251, type: !3, isLocal: true, isDefinition: true, scopeLine: 252,
> > isOptimized: false, unit: !5)
> > !2 = !DIFile(filename: "foo.c", directory: "/bar")
> > !3 = !DISubroutineType(types: !4)
> > !4 = !{}
> > !5 = distinct !DICompileUnit(language: DW_LANG_C, file: !2, producer:
> > "My compiler", isOptimized: false, runtimeVersion: 0, emissionKind:
> > LineTablesOnly)
> > LLVM ERROR: Broken module found, compilation aborted!
> >
> > As far as I can see in the printouts !1 is only used on func_27 and
> > still it complains.
> >
> > func27 was changed during argpromotion:
> >
> > ARG PROMOTION: Promoting to:
> > declare !dbg !0 internal i64 @func_27(i32*)
> >
> > From:
> > define internal i64 @func_27(i32* %p1, i32* %p2) {
> > store i32* %p1, i32** undef
> > store i32* @g_280, i32** undef
> > ret i64 undef
> > }
> >
> > so I'm not sure if the old func_27 is still hanging around in some way?
> >
> > Should I write a PR on this or do you understand what's going on?
> >
> > Regards,
> > Mikael
> >
> > On 07/06/2018 10:04 AM, Tim Northover via llvm-commits wrote:
> >> Author: tnorthover
> >> Date: Fri Jul 6 01:04:47 2018
> >> New Revision: 336419
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=336419&view=rev
> >> Log:
> >> CallGraphSCCPass: iterate over all functions.
> >>
> >> Previously we only iterated over functions reachable from the set of
> >> external functions in the module. But since some of the passes under
> >> this (notably the always-inliner and coroutine lowerer) are required for
> >> correctness, they need to run over everything.
> >>
> >> This just adds an extra layer of iteration over the CallGraph to keep
> >> track of which functions we've already visited and get the next batch of
> >> SCCs.
> >>
> >> Should fix PR38029.
> >>
> >> Modified:
> >> llvm/trunk/include/llvm/ADT/SCCIterator.h
> >> llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
> >> llvm/trunk/test/Transforms/Inline/always-inline.ll
> >>
> >> Modified: llvm/trunk/include/llvm/ADT/SCCIterator.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SCCIterator.h?rev=336419&r1=336418&r2=336419&view=diff
> >>
> >>
> ==============================================================================
>
> >>
> >> --- llvm/trunk/include/llvm/ADT/SCCIterator.h (original)
> >> +++ llvm/trunk/include/llvm/ADT/SCCIterator.h Fri Jul 6 01:04:47 2018
> >> @@ -90,6 +90,7 @@ class scc_iterator : public iterator_fac
> >> /// Compute the next SCC using the DFS traversal.
> >> void GetNextSCC();
> >> +public:
> >> scc_iterator(NodeRef entryN) : visitNum(0) {
> >> DFSVisitOne(entryN);
> >> GetNextSCC();
> >> @@ -98,12 +99,6 @@ class scc_iterator : public iterator_fac
> >> /// End is when the DFS stack is empty.
> >> scc_iterator() = default;
> >> -public:
> >> - static scc_iterator begin(const GraphT &G) {
> >> - return scc_iterator(GT::getEntryNode(G));
> >> - }
> >> - static scc_iterator end(const GraphT &) { return scc_iterator(); }
> >> -
> >> /// Direct loop termination test which is more efficient than
> >> /// comparison with \c end().
> >> bool isAtEnd() const {
> >> @@ -222,16 +217,25 @@ bool scc_iterator<GraphT, GT>::hasLoop()
> >> return false;
> >> }
> >> -/// Construct the begin iterator for a deduced graph type T.
> >> +/// Construct the begin iterator for a deduced graph type T, starting
> >> from its
> >> +/// entry node.
> >> template <class T> scc_iterator<T> scc_begin(const T &G) {
> >> - return scc_iterator<T>::begin(G);
> >> + return scc_iterator<T>(GraphTraits<T>::getEntryNode(G));
> >> }
> >> -/// Construct the end iterator for a deduced graph type T.
> >> +/// Construct the begin iterator for a graph type T, starting from
> >> the specified
> >> +/// node.
> >> +template <class T, class U = GraphTraits<T>>
> >> +scc_iterator<T, U> scc_begin(typename U::NodeRef N) {
> >> + return scc_iterator<T>(N);
> >> +}
> >> +
> >> + /// Construct the end iterator for a deduced graph type T.
> >> template <class T> scc_iterator<T> scc_end(const T &G) {
> >> - return scc_iterator<T>::end(G);
> >> + return scc_iterator<T>();
> >> }
> >> +
> >> } // end namespace llvm
> >> #endif // LLVM_ADT_SCCITERATOR_H
> >>
> >> Modified: llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp?rev=336419&r1=336418&r2=336419&view=diff
> >>
> >>
> ==============================================================================
>
> >>
> >> --- llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp (original)
> >> +++ llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp Fri Jul 6 01:04:47
> 2018
> >> @@ -17,6 +17,7 @@
> >> #include "llvm/Analysis/CallGraphSCCPass.h"
> >> #include "llvm/ADT/DenseMap.h"
> >> +#include "llvm/ADT/DenseSet.h"
> >> #include "llvm/ADT/SCCIterator.h"
> >> #include "llvm/ADT/Statistic.h"
> >> #include "llvm/Analysis/CallGraph.h"
> >> @@ -63,6 +64,10 @@ public:
> >> /// whether any of the passes modifies the module, and if so,
> >> return true.
> >> bool runOnModule(Module &M) override;
> >> + /// Execute all of the passes scheduled for execution on a given
> >> SCC. Return
> >> + /// true if any passes modify the module.
> >> + bool runOnSCC(CallGraphSCC &SCC, CallGraph &CG);
> >> +
> >> using ModulePass::doInitialization;
> >> using ModulePass::doFinalization;
> >> @@ -448,51 +453,78 @@ bool CGPassManager::RunAllPassesOnSCC(Ca
> >> bool CGPassManager::runOnModule(Module &M) {
> >> CallGraph &CG = getAnalysis<CallGraphWrapperPass>().getCallGraph();
> >> bool Changed = doInitialization(CG);
> >> -
> >> - // Walk the callgraph in bottom-up SCC order.
> >> - scc_iterator<CallGraph*> CGI = scc_begin(&CG);
> >> - CallGraphSCC CurSCC(CG, &CGI);
> >> - while (!CGI.isAtEnd()) {
> >> - // Copy the current SCC and increment past it so that the pass
> >> can hack
> >> - // on the SCC if it wants to without invalidating our iterator.
> >> - const std::vector<CallGraphNode *> &NodeVec = *CGI;
> >> - CurSCC.initialize(NodeVec);
> >> - ++CGI;
> >> -
> >> - // At the top level, we run all the passes in this pass manager
> >> on the
> >> - // functions in this SCC. However, we support iterative
> >> compilation in the
> >> - // case where a function pass devirtualizes a call to a
> >> function. For
> >> - // example, it is very common for a function pass (often GVN or
> >> instcombine)
> >> - // to eliminate the addressing that feeds into a call. With that
> >> improved
> >> - // information, we would like the call to be an inline candidate,
> >> infer
> >> - // mod-ref information etc.
> >> - //
> >> - // Because of this, we allow iteration up to a specified
> >> iteration count.
> >> - // This only happens in the case of a devirtualized call, so we
> >> only burn
> >> - // compile time in the case that we're making progress. We also
> >> have a hard
> >> - // iteration count limit in case there is crazy code.
> >> - unsigned Iteration = 0;
> >> - bool DevirtualizedCall = false;
> >> - do {
> >> - LLVM_DEBUG(if (Iteration) dbgs()
> >> - << " SCCPASSMGR: Re-visiting SCC, iteration #" <<
> >> Iteration
> >> - << '\n');
> >> - DevirtualizedCall = false;
> >> - Changed |= RunAllPassesOnSCC(CurSCC, CG, DevirtualizedCall);
> >> - } while (Iteration++ < MaxIterations && DevirtualizedCall);
> >> -
> >> - if (DevirtualizedCall)
> >> - LLVM_DEBUG(dbgs() << " CGSCCPASSMGR: Stopped iteration after "
> >> - << Iteration
> >> - << " times, due to -max-cg-scc-iterations\n");
> >> + DenseSet<const Function *> Visited;
> >> - MaxSCCIterations.updateMax(Iteration);
> >> + CallGraph::iterator INext;
> >> + for (auto I = CG.begin(), E = CG.end(); I != E;) {
> >> + // Walk the callgraph in bottom-up SCC order.
> >> + auto CGI = scc_begin<CallGraph *>(I->second.get());
> >> +
> >> + CallGraphSCC CurSCC(CG, &CGI);
> >> + while (!CGI.isAtEnd()) {
> >> + const std::vector<CallGraphNode *> &NodeVec = *CGI;
> >> +
> >> + // Record that we've seen this set of functions so we don't run
> >> the pass
> >> + // twice.
> >> + for (auto *Elt : NodeVec)
> >> + Visited.insert(Elt->getFunction());
> >> +
> >> + // Copy the current SCC and increment past it so that the pass
> >> can hack
> >> + // on the SCC if it wants to without invalidating our iterator.
> >> + CurSCC.initialize(NodeVec);
> >> + ++CGI;
> >> +
> >> + // If we've got all functions reachable from our chosen initial
> >> node,
> >> + // calculate the next node we need to search from now before
> >> parts of the
> >> + // graph are invalidated.
> >> + if (CGI.isAtEnd()) {
> >> + do
> >> + ++I;
> >> + while (I != E && Visited.count(I->first));
> >> + }
> >> +
> >> + Changed |= runOnSCC(CurSCC, CG);
> >> + }
> >> }
> >> +
> >> Changed |= doFinalization(CG);
> >> return Changed;
> >> }
> >> +bool CGPassManager::runOnSCC(CallGraphSCC &CurSCC, CallGraph &CG) {
> >> + bool Changed = false;
> >> +
> >> + // At the top level, we run all the passes in this pass manager on
> the
> >> + // functions in this SCC. However, we support iterative
> >> compilation in the
> >> + // case where a function pass devirtualizes a call to a function.
> For
> >> + // example, it is very common for a function pass (often GVN or
> >> instcombine)
> >> + // to eliminate the addressing that feeds into a call. With that
> >> improved
> >> + // information, we would like the call to be an inline candidate,
> >> infer
> >> + // mod-ref information etc.
> >> + //
> >> + // Because of this, we allow iteration up to a specified iteration
> >> count.
> >> + // This only happens in the case of a devirtualized call, so we
> >> only burn
> >> + // compile time in the case that we're making progress. We also
> >> have a hard
> >> + // iteration count limit in case there is crazy code.
> >> + unsigned Iteration = 0;
> >> + bool DevirtualizedCall = false;
> >> + do {
> >> + LLVM_DEBUG(if (Iteration) dbgs()
> >> + << " SCCPASSMGR: Re-visiting SCC, iteration #" <<
> >> Iteration
> >> + << '\n');
> >> + DevirtualizedCall = false;
> >> + Changed |= RunAllPassesOnSCC(CurSCC, CG, DevirtualizedCall);
> >> + } while (Iteration++ < MaxIterations && DevirtualizedCall);
> >> +
> >> + if (DevirtualizedCall)
> >> + LLVM_DEBUG(dbgs() << " CGSCCPASSMGR: Stopped iteration after "
> >> << Iteration
> >> + << " times, due to -max-cg-scc-iterations\n");
> >> +
> >> + MaxSCCIterations.updateMax(Iteration);
> >> + return Changed;
> >> +}
> >> +
> >> /// Initialize CG
> >> bool CGPassManager::doInitialization(CallGraph &CG) {
> >> bool Changed = false;
> >>
> >> Modified: llvm/trunk/test/Transforms/Inline/always-inline.ll
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=336419&r1=336418&r2=336419&view=diff
> >>
> >>
> ==============================================================================
>
> >>
> >> --- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)
> >> +++ llvm/trunk/test/Transforms/Inline/always-inline.ll Fri Jul 6
> >> 01:04:47 2018
> >> @@ -316,3 +316,10 @@ define void @outer14() {
> >> call void @inner14()
> >> ret void
> >> }
> >> +
> >> +define internal i32 @outer15() {
> >> +; CHECK-LABEL: @outer15
> >> +; CHECK: ret i32 1
> >> + %res = call i32 @inner1()
> >> + ret i32 %res
> >> +}
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180710/9fa59fcf/attachment-0001.html>
More information about the llvm-commits
mailing list