[llvm] r336419 - CallGraphSCCPass: iterate over all functions.
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 9 22:42:49 PDT 2018
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
>
More information about the llvm-commits
mailing list