[llvm] r360171 - [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor

Fri May 10 16:26:14 PDT 2019

> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190511/bc6619e6/attachment.html>