[llvm] r347917 - [DAGCombiner] narrow truncated binops

Thu Dec 6 16:26:26 PST 2018

DAGCombiner::Run makes sure all nodes are visited at least once. It does
this by checking the operands for everything that's visited. So if the node
that's being added to the worklist was created just before the add to
worklist then I don't think it needs to be explicitly added.

~Craig

On Thu, Dec 6, 2018 at 4:13 PM Sanjay Patel <spatel at rotateright.com> wrote:

> Ok...there are still some seemingly obvious bugs in
> hoistLogicOpWithSameOpcodeHands(), so I'm going to keep trying to kill
> those.
>
> But I've hit a couple of things I don't understand:
> 1. Does that code need to explicitly call AddToWorklist()?
> 2. How did this happen?
> https://reviews.llvm.org/rL348552
>
> On Thu, Dec 6, 2018 at 3:52 PM David Jones <dlj at google.com> wrote:
>
>> Sadly, I don't think r348508 fixes the issue. I still see the large
>> memory growth. I collected some more traces from the allocator, and it
>> looks like there is still quite a bit of allocation happening
>> in hoistLogicOpWithSameOpcodeHands, but most of the growth is actually
>> coming from further up the stack (SelectionDAG::Combine).
>>
>> On Thu, Dec 6, 2018 at 10:27 AM Sanjay Patel <spatel at rotateright.com>
>> wrote:
>>
>>> First attempt at damage control in that function:
>>> https://reviews.llvm.org/rL348508
>>>
>>> If I've guessed correctly, that's enough to prevent the OOM. Still
>>> looking at fixing that better though.
>>>
>>>
>>> On Thu, Dec 6, 2018 at 8:12 AM Sanjay Patel <spatel at rotateright.com>
>>> wrote:
>>>
>>>> #10 0x000055680f8d7466 (anonymous
>>>> namespace)::DAGCombiner::SimplifyBinOpWithSameOpcodeHands(llvm::SDNode*)
>>>>
>>>> Thanks! That's a good lead. I'm looking at that code now, and it
>>>> doesn't appear to check the number of uses before doing transforms. That
>>>> means it can increase the instruction count at each invocation. Let me try
>>>> some experiments in there.
>>>>
>>>> On Wed, Dec 5, 2018 at 7:51 PM David Jones <dlj at google.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Wed, Dec 5, 2018 at 6:34 PM Friedman, Eli <efriedma at codeaurora.org>
>>>>> wrote:
>>>>>
>>>>>> On 12/5/2018 5:47 PM, David Jones wrote:
>>>>>>
>>>>>>
>>>>>> On Wed, Dec 5, 2018 at 5:40 PM Sanjay Patel <spatel at rotateright.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi David,
>>>>>>>
>>>>>>> Thanks for reporting the problem. I don’t have any guesses as to how
>>>>>>> this could cause OOM. Cc’ing some people that might.
>>>>>>>
>>>>>>
>>>>>> +a few more, thanks! :-)
>>>>>>
>>>>>>
>>>>>>> If the problem is only showing up on PPC, a quick hack would be to
>>>>>>> add a TLI hook and disable the transform for PPC. I’d prefer that to a full
>>>>>>> revert (especially since I’ve already enhanced this code to handle some
>>>>>>> vectors, and another enhancement is in progress)...unless we have a test
>>>>>>> case that shows it’s not a PPC-specific bug?
>>>>>>>
>>>>>>>
>>>>>> Agreed on avoiding a revert... this looks like a pretty hard-fought
>>>>>> change, based on https://bugs.llvm.org/show_bug.cgi?id=32023.
>>>>>>
>>>>>> I do believe this is PPC-specific (although, as is often the case,
>>>>>> your change may simply be tickling a bug somewhere else).
>>>>>>
>>>>>> If it's PPC-specific, my first guess here would be there's some
>>>>>> PowerPC-specific target combine that's trying to combine the opposite way,
>>>>>> or something like that.  Assuming that's the case, it probably runs out of
>>>>>> memory pretty quickly, so it should be fast to bisect if you restrict the
>>>>>> memory usage with ulimit.
>>>>>>
>>>>>
>>>>> So I did try something like this... instead of just ulimit, I patched
>>>>> my allocator to crash when it sees a sufficiently large allocation (so I
>>>>> can see the large allocation itself, not just a random allocation when
>>>>> close to the limit).
>>>>>
>>>>> Here's a stacktrace I got (slightly edited for clarity):
>>>>>
>>>>> 1. <eof> parser at end of file
>>>>> 2. Code generation
>>>>> 3. Running pass 'Function Pass Manager' on module
>>>>> 'FrontendAction-9a7810.cpp'.
>>>>> 4. Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on
>>>>> function
>>>>> '@_ZN5clang14FrontendAction15BeginSourceFileERNS_16CompilerInstanceERKNS_17FrontendInputFileE'
>>>>> #0 0x000055681055d388 llvm::sys::RunSignalHandlers()
>>>>> #1 0x000055681055f486 SignalHandler(int)
>>>>> #2 0x00007f88bd9f89a0 __restore_rt
>>>>> #3 0x00007f88bd77a602 __GI_raise
>>>>> #4 0x00007f88bd77c320 __GI_abort
>>>>> #5 0x00005568105e2a70 tcmalloc::Logger::Add(tcmalloc::LogItem const&)
>>>>> #6 0x00005568105d5838 (anonymous namespace)::do_malloc_pages(unsigned
>>>>> long, unsigned long)
>>>>> #7 0x000055681068a56d realloc
>>>>> #8 0x000055681055c789 llvm::SmallVectorBase::grow_pod(void*, unsigned
>>>>> long, unsigned long)
>>>>> #9 0x000055680f895119 (anonymous
>>>>> namespace)::DAGCombiner::AddToWorklist(llvm::SDNode*)
>>>>> #10 0x000055680f8d7466 (anonymous
>>>>> namespace)::DAGCombiner::SimplifyBinOpWithSameOpcodeHands(llvm::SDNode*)
>>>>> #11 0x000055680f8a1791 (anonymous
>>>>> namespace)::DAGCombiner::visitAND(llvm::SDNode*)
>>>>> #12 0x000055680f89729b (anonymous
>>>>> namespace)::DAGCombiner::combine(llvm::SDNode*)
>>>>> #13 0x000055680f8960cf llvm::SelectionDAG::Combine(llvm::CombineLevel,
>>>>> llvm::AAResults*, llvm::CodeGenOpt::Level)
>>>>> #14 0x000055680f78cdbc llvm::SelectionDAGISel::CodeGenAndEmitDAG()
>>>>> #15 0x000055680f78bbde
>>>>> llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)
>>>>> #16 0x000055680f788c56
>>>>> llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
>>>>> #17 0x000055680f418574 (anonymous
>>>>> namespace)::PPCDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&)
>>>>> #18 0x000055680fa0955b
>>>>> llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
>>>>> #19 0x0000556810478c6c
>>>>> llvm::FPPassManager::runOnFunction(llvm::Function&)
>>>>> #20 0x0000556810478f23 llvm::FPPassManager::runOnModule(llvm::Module&)
>>>>> #21 0x00005568104793f5
>>>>> llvm::legacy::PassManagerImpl::run(llvm::Module&)
>>>>> #22 0x000055680da6c9ff
>>>>> 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> >)
>>>>> #23 0x000055680da43542
>>>>> clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)
>>>>> #24 0x000055680e47c7b3 clang::ParseAST(clang::Sema&, bool, bool)
>>>>> #25 0x000055680e2bc0fa clang::FrontendAction::Execute()
>>>>> #26 0x000055680e142428
>>>>> clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
>>>>> #27 0x000055680d80b562
>>>>> clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
>>>>> #28 0x000055680d7fab05 cc1_main(llvm::ArrayRef<char const*>, char
>>>>> const*, void*)
>>>>> #29 0x000055680d8088ea main
>>>>> #30 0x00007f88bd766bbd __libc_start_main
>>>>> #31 0x000055680d7fa469 _start
>>>>>
>>>>>
>>>>> (I've been poking at this in a few other ways... the stacks vary
>>>>> somewhat, but they all seem to point to an ever-growing work list
>>>>> in DAGCombiner, inside of PPCDAGToDAGISel -- so #13-#31 seem to be the
>>>>> common parts of the stacks.)
>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181206/c5b4c9e8/attachment.html>