[llvm] r336419 - CallGraphSCCPass: iterate over all functions.
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 13 09:37:38 PDT 2018
Reverted in r337018.
On Fri, Jul 13, 2018 at 9:34 AM, Evgenii Stepanov <eugeni.stepanov at gmail.com
> wrote:
> Thank you. So an SCC pass (Inliner in this case) may invalidate the
> CallGraph iterator held in the loop in CGPassManager::runOnModule by
> removing a function from the module. This is clearly a bug in this change,
> reverting.
>
>
>
> On Fri, Jul 13, 2018 at 9:00 AM, Sumanth Gundapaneni <
> sgundapa at codeaurora.org> wrote:
>
>> I was able to reproduce the problem consistently with my ASAN build.
>>
>> I cannot share the source code but I can share the crash log.
>>
>> It is a “heap-use-after-free” that comes from debug statement (#ifndef
>> NDEBUG).
>>
>> Hopefully this helps.
>>
>>
>>
>> --Sumanth
>>
>> *From:* llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] *On
>> Behalf Of *Evgenii Stepanov via llvm-commits
>> *Sent:* Friday, July 13, 2018 10:24 AM
>> *To:* Galina Kistanova <gkistanova at gmail.com>
>> *Cc:* LLVM Commits <llvm-commits at lists.llvm.org>
>> *Subject:* Re: [llvm] r336419 - CallGraphSCCPass: iterate over all
>> functions.
>>
>>
>>
>> MSan buildbot crashes are still happening, once per 8 builds or so. I
>> can't reproduce them locally and am not 100% sure that this is the change,
>> but, given the nondeterminism detected on the other bot, I'm going to
>> tentatively revert it.
>>
>>
>>
>>
>>
>> On Thu, Jul 12, 2018 at 12:01 PM, Galina Kistanova <gkistanova at gmail.com>
>> wrote:
>>
>> Hello Tim,
>>
>>
>>
>> This commit has introduced nondeterminism. Please see
>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu or
>> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu build bots.
>>
>>
>>
>> This is in addition to those other problems mentioned already, or maybe
>> some of the other problems are exposed because of this.
>>
>>
>>
>> Since the problem is around for quite long now, maybe it worth reverting
>> till you will figure out what's wrong and how to fix it?
>>
>>
>>
>> Thanks
>>
>>
>>
>> Galina
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Jul 10, 2018 at 1:33 PM, Evgenii Stepanov via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>> Hi Tim,
>>
>>
>>
>> MSan bootstrap bot has been randomly crashing lately with the following
>> trace:
>>
>>
>>
>> 3. Running pass 'CallGraph Pass Manager' on module
>> '/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/lib/Tar
>> get/Hexagon/HexagonCommonGEP.cpp'.
>>
>> #0 0x00005602f79f5cfa llvm::sys::PrintStackTrace(llvm::raw_ostream&)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x24f5cfa)
>>
>> #1 0x00005602f79f42c5 llvm::sys::RunSignalHandlers()
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x24f42c5)
>>
>> #2 0x00005602f79f43dc SignalHandler(int) (/b/sanitizer-x86_64-linux-boo
>> tstrap-msan/build/llvm_build0/bin/clang-7+0x24f43dc)
>>
>> #3 0x00007f63daa840c0 __restore_rt (/lib/x86_64-linux-gnu/libpthr
>> ead.so.0+0x110c0)
>>
>> #4 0x00005602f6edf38e llvm::scc_iterator<llvm::CallGraph*,
>> llvm::GraphTraits<llvm::CallGraph*> >::DFSVisitChildren()
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x19df38e)
>>
>> #5 0x00005602f6edf5aa llvm::scc_iterator<llvm::CallGraph*,
>> llvm::GraphTraits<llvm::CallGraph*> >::GetNextSCC()
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x19df5aa)
>>
>> #6 0x00005602f6ee05ed (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x19e05ed)
>>
>> #7 0x00005602f74ca5fd llvm::legacy::PassManagerImpl::run(llvm::Module&)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x1fca5fd)
>>
>> #8 0x00005602f7bde5c1 (anonymous namespace)::EmitAssemblyHelper
>> ::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream,
>> std::default_delete<llvm::raw_pwrite_stream> >)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x26de5c1)
>>
>> #9 0x00005602f7bdfdf0 clang::EmitBackendOutput(clang::DiagnosticsEngine&,
>> clang::HeaderSearchOptions const&, clang::CodeGenOptions const&,
>> clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout
>> const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream,
>> std::default_delete<llvm::raw_pwrite_stream> >)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x26dfdf0)
>>
>> #10 0x00005602f836f264 clang::BackendConsumer::Handle
>> TranslationUnit(clang::ASTContext&) (/b/sanitizer-x86_64-linux-boo
>> tstrap-msan/build/llvm_build0/bin/clang-7+0x2e6f264)
>>
>> #11 0x00005602f8b37fa9 clang::ParseAST(clang::Sema&, bool, bool)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x3637fa9)
>>
>> #12 0x00005602f836e4d9 clang::CodeGenAction::ExecuteAction()
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x2e6e4d9)
>>
>> #13 0x00005602f800da7e clang::FrontendAction::Execute()
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x2b0da7e)
>>
>> #14 0x00005602f7fd9496 clang::CompilerInstance::Execu
>> teAction(clang::FrontendAction&) (/b/sanitizer-x86_64-linux-boo
>> tstrap-msan/build/llvm_build0/bin/clang-7+0x2ad9496)
>>
>> #15 0x00005602f80ad511 clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
>> (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0x2bad511)
>>
>> #16 0x00005602f614c080 cc1_main(llvm::ArrayRef<char const*>, char const*,
>> void*) (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build0/
>> bin/clang-7+0xc4c080)
>>
>> #17 0x00005602f60c1a22 main (/b/sanitizer-x86_64-linux-boo
>> tstrap-msan/build/llvm_build0/bin/clang-7+0xbc1a22)
>>
>> #18 0x00007f63d98312e1 __libc_start_main (/lib/x86_64-linux-gnu/libc.so
>> .6+0x202e1)
>>
>> #19 0x00005602f61480aa _start (/b/sanitizer-x86_64-linux-boo
>> tstrap-msan/build/llvm_build0/bin/clang-7+0xc480aa)
>>
>> clang-7: error: unable to execute command: Segmentation fault
>>
>>
>>
>> The crash happens roughly in 1 out of 5-to-10 builds, see history here:
>>
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-
>> bootstrap-msan?numbuilds=300
>>
>>
>>
>> The earliest it happened was this build at r336432:
>>
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-
>> bootstrap-msan/builds/5763
>>
>>
>>
>> I suspect that this change could be the culprit. Could you take a look?
>>
>>
>>
>>
>>
>> On Fri, Jul 6, 2018 at 1:04 AM, Tim Northover via llvm-commits <
>> llvm-commits at lists.llvm.org> 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/Transfor
>> ms/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
>>
>>
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180713/0a0a225e/attachment.html>
More information about the llvm-commits
mailing list