[llvm] r360171 - [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 4 10:29:11 PDT 2019
Sorry for the delay, just got back from vacation today! Looks like the
patch is committed, I'll go run some tests now.
On Wed, May 29, 2019 at 4:02 PM Florian Hahn <florian_hahn at apple.com> wrote:
> 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
>
> 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>
> 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 = a.am = a.an = a.ap = a.c = a.aa = h.aa;
>> 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>
>> *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> 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>
>>> *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>
>>>> *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>
>>>>> *Date: *Tue, May 7, 2019 at 9:45 AM
>>>>> *To: *<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
>>>>>> 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
>>>>>>
>>>>>> 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
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- 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
>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>
>>>>> <repro.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/20190604/f7b880d2/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190604/f7b880d2/attachment-0001.bin>
More information about the llvm-commits
mailing list