[llvm] r360171 - [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Wed May 29 16:01:37 PDT 2019
Hi Jordan,
Thanks for the reproducers (and sorry for my late response)! The problem was indeed caused by DAGCombiner. I’ve put up an updated version of the original patch: https://reviews.llvm.org/D62633 <https://reviews.llvm.org/D62633>
It does not crash the C and .ll reproducer. Would it be possible to check if it fixes the issue on the unreduced code?
Cheers,
Florian
> On May 21, 2019, at 4:12 PM, Jordan Rupprecht via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Hi Florian,
> Sorry for the delay with this second reproducer. c-reduce was turtle slow and didn't take it very far, and bugpoint didn't like it either and needed some manual reducing. The .ll reproducer I have is still >5k lines :(
>
> Repro instructions (when synced prior to reverting this commit; repro.ll is attached):
> $ time ~/dev/llc repro.ll -o /tmp/repro.o
>
> llc: /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:132: void llvm::LiveVariables::HandleVirtRegUse(unsigned int, llvm::MachineBasicBlock *, llvm::MachineInstr &): Assertion `MRI->getVRegDef(reg) && "Register use before def!"' failed.
> Stack dump:
> 0. Program arguments: /usr/local/google/home/rupprecht/dev/llc repro.ll -o /tmp/repro.o
> 1. Running pass 'Function Pass Manager' on module 'repro.ll'.
> 2. Running pass 'Live Variable Analysis' on function '@repro'
> #0 0x00007f5915e1136f llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:494:13
> #1 0x00007f5915e0f5d0 llvm::sys::RunSignalHandlers() /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> #2 0x00007f5915e11778 SignalHandler(int) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:357:1
> #3 0x00007f59156cc0c0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0)
> #4 0x00007f5914aadfcf raise (/lib/x86_64-linux-gnu/libc.so.6+0x32fcf)
> #5 0x00007f5914aaf3fa abort (/lib/x86_64-linux-gnu/libc.so.6+0x343fa)
> #6 0x00007f5914aa6e37 (/lib/x86_64-linux-gnu/libc.so.6+0x2be37)
> #7 0x00007f5914aa6ee2 (/lib/x86_64-linux-gnu/libc.so.6+0x2bee2)
> #8 0x00007f5916de41ad (/usr/local/google/home/rupprecht/src/llvm-build/dev/bin/../lib/libLLVMCodeGen.so.9svn+0x34e1ad)
> #9 0x00007f5916de641e llvm::LiveVariables::runOnInstr(llvm::MachineInstr&, llvm::SmallVectorImpl<unsigned int>&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:542:46
> #10 0x00007f5916de6b0c llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false>::isEnd() const /usr/local/google/home/rupprecht/src/llvm-project/llvm/include/llvm/ADT/ilist_iterator.h:176:31
> #11 0x00007f5916de6b0c llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false> llvm::MachineInstrBundleIteratorHelper<false>::getBundleFinal<llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false> >(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false>) /usr/local/google/home/rupprecht/src/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h:62:0
> #12 0x00007f5916de6b0c void llvm::MachineInstrBundleIteratorHelper<false>::increment<llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false> >(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false>&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h:70:0
> #13 0x00007f5916de6b0c llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>::operator++() /usr/local/google/home/rupprecht/src/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h:260:0
> #14 0x00007f5916de6b0c llvm::LiveVariables::runOnBlock(llvm::MachineBasicBlock*, unsigned int) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:577:0
> #15 0x00007f5916de7201 llvm::LiveVariables::runOnMachineFunction(llvm::MachineFunction&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:650:32
> #16 0x00007f5916e4abea llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:13
> #17 0x00007f59169fcc43 llvm::FPPassManager::runOnFunction(llvm::Function&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1648:27
> #18 0x00007f59169fcf53 llvm::FPPassManager::runOnModule(llvm::Module&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1685:13
> #19 0x00007f59169fd3e8 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1752:27
> #20 0x00007f59169fd3e8 llvm::legacy::PassManagerImpl::run(llvm::Module&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1865:0
> #21 0x000000000021585c compileModule(char**, llvm::LLVMContext&) /usr/local/google/home/rupprecht/src/llvm-project/llvm/tools/llc/llc.cpp:609:8
> #22 0x00000000002132dd main /usr/local/google/home/rupprecht/src/llvm-project/llvm/tools/llc/llc.cpp:363:13
>
> On Fri, May 10, 2019 at 11:09 PM Jordan Rupprecht <rupprecht at google.com <mailto:rupprecht at google.com>> wrote:
> Attached a repro.cc from creduce. Repro is just:
> $ cat repro.cc
> struct b {
> char aa;
> };
> struct __attribute__((__packed__)) d {
> char : 84211;
> char ak : 1;
> int al : 32;
> int am : 32;
> char an : 5;
> char : 5;
> char ap : 5;
> short c : 6;
> int aa : 27;
> int at : 32;
> int a;
> void e();
> };
> class f {
> void g(const b &);
> long az[];
> };
> void f::g(const b &h) {
> d a;
> a.ak = a.al <http://a.al/> = a.am <http://a.am/> = a.an <http://a.an/> = a.ap = a.c = a.aa = h.aa;
> a.at <http://a.at/> = az[0];
> a.e();
> }
> $ clang++ -O2 -fsanitize=address -o repro.o -c repro.cc
> And I just noticed that clang w/ debug prints this before the stack:
>
> clang-9: $SRC/llvm/lib/CodeGen/MachineTraceMetrics.cpp:639: (anonymous namespace)::DataDep::DataDep(const llvm::MachineRegisterInfo *, unsigned int, unsigned int): Assertion `!DefI.atEnd() && "Register has no defs"' failed.
>
> creduce is going a little more slowly on the code that crashed in "Live Variable Analysis", but I'll leave that going over the weekend. Hope this initial test case helps a bit.
>
> From: Florian Hahn <flo at fhahn.com <mailto:flo at fhahn.com>>
> Date: Fri, May 10, 2019 at 4:26 PM
> To: Jordan Rupprecht
> Cc: llvm-commits
>
>
>
> On 11 May 2019, at 00:22, Jordan Rupprecht <rupprecht at google.com <mailto:rupprecht at google.com>> wrote:
>
>> Reverted in r360481. I have creduce going on the Machine InstCombiner crash and down to "only" 7000 lines, so I think it will finish before I leave today.
>>
>> There were some crashes in other places (i.e. I don't think the bug is in this patch, it's just revealing bugs in other places), so I'll have to see if those reduce to the same thing -- I'll check that out next week.
>>
>> Sorry for the inconvenience!
>>
>
> No worries, thanks for letting me know. I’m curious what the problem with the machine combiner is.
>
> Cheers
>> From: Jordan Rupprecht <rupprecht at google.com <mailto:rupprecht at google.com>>
>> Date: Fri, May 10, 2019 at 3:28 PM
>> To: Florian Hahn
>> Cc: llvm-commits
>>
>> Here's the full crash (minus the boiler plate) for the one in Machine InstCombiner:
>> #9 0x00007fb8d52cf5d2 (anonymous namespace)::DataDep::DataDep(llvm::MachineRegisterInfo const*, unsigned int, unsigned int) ${LLVM_SRC}/llvm/lib/CodeGen/MachineTraceMetrics.cpp:640:13
>> #10 0x00007fb8d52cb644 getDataDeps(llvm::MachineInstr const&, llvm::SmallVectorImpl<(anonymous namespace)::DataDep>&, llvm::MachineRegisterInfo const*) ${LLVM_SRC}/llvm/lib/CodeGen/MachineTraceMetrics.cpp:672:22
>> #11 0x00007fb8d52cb064 llvm::MachineTraceMetrics::Ensemble::updateDepth(llvm::MachineTraceMetrics::TraceBlockInfo&, llvm::MachineInstr const&, llvm::SparseSet<llvm::LiveRegUnit, llvm::identity<unsigned int>, unsigned char>&) ${LLVM_SRC}/llvm/lib/CodeGen/MachineTraceMetrics.cpp:788:12
>> #12 0x00007fb8d52cc300 llvm::MachineTraceMetrics::Ensemble::computeInstrDepths(llvm::MachineBasicBlock const*) ${LLVM_SRC}/llvm/lib/CodeGen/MachineTraceMetrics.cpp:883:28
>> #13 0x00007fb8d52cde9e llvm::MachineTraceMetrics::Ensemble::getTrace(llvm::MachineBasicBlock const*) ${LLVM_SRC}/llvm/lib/CodeGen/MachineTraceMetrics.cpp:1166:8
>> #14 0x00007fb8d515ead4 (anonymous namespace)::MachineCombiner::combineInstructions(llvm::MachineBasicBlock*) ${LLVM_SRC}/llvm/lib/CodeGen/MachineCombiner.cpp:596:59
>> #15 0x00007fb8d515e342 (anonymous namespace)::MachineCombiner::runOnMachineFunction(llvm::MachineFunction&) ${LLVM_SRC}/llvm/lib/CodeGen/MachineCombiner.cpp:654:16
>> #16 0x00007fb8d51bf637 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) ${LLVM_SRC}/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:8
>> #17 0x00007fb8d47dd759 llvm::FPPassManager::runOnFunction(llvm::Function&) ${LLVM_SRC}/llvm/lib/IR/LegacyPassManager.cpp:1648:23
>> #18 0x00007fb8d47ddb9f llvm::FPPassManager::runOnModule(llvm::Module&) ${LLVM_SRC}/llvm/lib/IR/LegacyPassManager.cpp:1685:16
>> #19 0x00007fb8d47de305 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) ${LLVM_SRC}/llvm/lib/IR/LegacyPassManager.cpp:1752:23
>> #20 0x00007fb8d47dde45 llvm::legacy::PassManagerImpl::run(llvm::Module&) ${LLVM_SRC}/llvm/lib/IR/LegacyPassManager.cpp:1865:16
>> #21 0x00007fb8d47de881 llvm::legacy::PassManager::run(llvm::Module&) ${LLVM_SRC}/llvm/lib/IR/LegacyPassManager.cpp:1896:3
>> #22 0x00007fb8d117e15a (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) ${LLVM_SRC}/clang/lib/CodeGen/BackendUtil.cpp:894:3
>>
>>
>> From: Jordan Rupprecht <rupprecht at google.com <mailto:rupprecht at google.com>>
>> Date: Fri, May 10, 2019 at 12:58 PM
>> To: Florian Hahn
>> Cc: llvm-commits
>>
>> Looks like this causes some asan/msan crashes (or in some cases, test failure/miscompiles). I'll see if I can get a crash with debug+asserts enabled. In release mode, it's crashing during Machine InstCombiner/Live Variable Analysis for different repros.
>>
>>
>> From: Florian Hahn via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
>> Date: Tue, May 7, 2019 at 9:45 AM
>> To: <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
>>
>> Author: fhahn
>> Date: Tue May 7 09:47:27 2019
>> New Revision: 360171
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=360171&view=rev <http://llvm.org/viewvc/llvm-project?rev=360171&view=rev>
>> Log:
>> [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor
>>
>> When simplifying TokenFactors, we potentially iterate over all
>> operands of a large number of TokenFactors. This causes quadratic
>> compile times in some cases and the large token factors cause additional
>> scalability problems elsewhere.
>>
>> This patch adds some limits to the number of nodes explored for the
>> cases mentioned above.
>>
>> Reviewers: niravd, spatel, craig.topper
>>
>> Reviewed By: niravd
>>
>> Differential Revision: https://reviews.llvm.org/D61397 <https://reviews.llvm.org/D61397>
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=360171&r1=360170&r2=360171&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=360171&r1=360170&r2=360171&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue May 7 09:47:27 2019
>> @@ -1792,8 +1792,9 @@ SDValue DAGCombiner::visitTokenFactor(SD
>> TFs.push_back(N);
>>
>> // Iterate through token factors. The TFs grows when new token factors are
>> - // encountered.
>> - for (unsigned i = 0; i < TFs.size(); ++i) {
>> + // encountered. Limit number of nodes to inline, to avoid quadratic compile
>> + // times.
>> + for (unsigned i = 0; i < TFs.size() && Ops.size() <= 2048; ++i) {
>> SDNode *TF = TFs[i];
>>
>> // Check each of the operands.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> <repro.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://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/20190529/ba6c95ee/attachment.html>
More information about the llvm-commits
mailing list