[llvm] r347917 - [DAGCombiner] narrow truncated binops

Sat Dec 8 08:27:52 PST 2018

Thanks to all for the help, and sorry about the bug. I've tried again with
a fix to avoid opaque constants here:
https://reviews.llvm.org/rL348706

On Fri, Dec 7, 2018 at 4:30 PM Sanjay Patel <spatel at rotateright.com> wrote:

> Thanks! The problem is obvious with the test cases - I didn't consider the
> possibility of opaque constants. I reduced a test from the Chrome repro,
> and I'll try to get one from the PPC file too.
>
> On Fri, Dec 7, 2018 at 2:08 PM David Jones <dlj at google.com> wrote:
>
>> I've attached my PPC-specific reproducer.
>>
>> Attached is the output, run with my malloc instrumented to crash on large
>> allocations.
>>
>> $
>> /google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang
>> -cc1 -triple powerpc64le-unknown-linux-gnu -fsanitize=return
>> -fexperimental-new-pass-manager -x c++ PPMacroExpansion-20d431.cpp
>> -emit-obj -o /dev/null -O1 -fdebug-pass-manager > out.txt 2>&1
>> [snip; full output attached]
>> Stack dump:
>> 0. Program arguments:
>> /google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang
>> -cc1 -triple powerpc64le-unknown-linux-gnu -fsanitize=return
>> -fexperimental-new-pass-manager -x c++ PPMacroExpansion-20d431.cpp
>> -emit-obj -o /dev/null -O1 -fdebug-pass-manager
>> 1. <eof> parser at end of file
>> 2. Code generation
>> 3. Running pass 'Function Pass Manager' on module
>> 'PPMacroExpansion-20d431.cpp'.
>> 4. Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on
>> function '@_Z1b1k'
>> #0 0x000055fc11234b88 llvm::sys::RunSignalHandlers()
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x31b6b88)
>> #1 0x000055fc11236c86 SignalHandler(int)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x31b8c86)
>> #2 0x00007f623763b9a0 __restore_rt
>> (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)
>> #3 0x00007f62373bd602 __GI_raise (/usr/grte/v4/lib64/libc.so.6+0x4c602)
>> #4 0x00007f62373bf320 __GI_abort (/usr/grte/v4/lib64/libc.so.6+0x4e320)
>> #5 0x000055fc112ba390 tcmalloc::Logger::Add(tcmalloc::LogItem const&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x323c390)
>> #6 0x000055fc112ad038 (anonymous namespace)::do_malloc_pages(unsigned
>> long, unsigned long)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x322f038)
>> #7 0x000055fc11361e6d realloc
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x32e3e6d)
>> #8 0x000055fc11233f89 llvm::SmallVectorBase::grow_pod(void*, unsigned
>> long, unsigned long)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x31b5f89)
>> #9 0x000055fc1056ae82 llvm::SelectionDAG::Combine(llvm::CombineLevel,
>> llvm::AAResults*, llvm::CodeGenOpt::Level)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x24ece82)
>> #10 0x000055fc104618bc llvm::SelectionDAGISel::CodeGenAndEmitDAG()
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x23e38bc)
>> #11 0x000055fc104606db
>> llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x23e26db)
>> #12 0x000055fc1045d746
>> llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x23df746)
>> #13 0x000055fc100ebf84 (anonymous
>> namespace)::PPCDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x206df84)
>> #14 0x000055fc106de3cb
>> llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x26603cb)
>> #15 0x000055fc1115036c
>> llvm::FPPassManager::runOnFunction(llvm::Function&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x30d236c)
>> #16 0x000055fc11150623 llvm::FPPassManager::runOnModule(llvm::Module&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x30d2623)
>> #17 0x000055fc11150af5 llvm::legacy::PassManagerImpl::run(llvm::Module&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x30d2af5)
>> #18 0x000055fc0e753528 (anonymous
>> namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction,
>> std::unique_ptr<llvm::raw_pwrite_stream,
>> std::default_delete<llvm::raw_pwrite_stream> >)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x6d5528)
>> #19 0x000055fc0e74ed1f
>> 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> >)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x6d0d1f)
>> #20 0x000055fc0e726f82
>> clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x6a8f82)
>> #21 0x000055fc0f159213 clang::ParseAST(clang::Sema&, bool, bool)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x10db213)
>> #22 0x000055fc0ef9bf1a clang::FrontendAction::Execute()
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0xf1df1a)
>> #23 0x000055fc0ee23008
>> clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0xda5008)
>> #24 0x000055fc0e4ef302
>> clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x471302)
>> #25 0x000055fc0e4de8c5 cc1_main(llvm::ArrayRef<char const*>, char const*,
>> void*)
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x4608c5)
>> #26 0x000055fc0e4ec6aa main
>> (/google/src/cloud/dlj/clang-crash3/google3/third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x46e6aa)
>> #27 0x00007f62373a9bbd __libc_start_main
>> (/usr/grte/v4/lib64/libc.so.6+0x38bbd)
>> #28 0x000055fc0e4de229 _start
>> /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:121:0
>>
>>
>>
>> On Fri, Dec 7, 2018 at 8:21 AM Hans Wennborg via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Sounds good. Thanks!
>>>
>>> On Fri, Dec 7, 2018 at 5:08 PM Sanjay Patel <spatel at rotateright.com>
>>> wrote:
>>> >
>>> > I added a debug flag to disable this transform by default, so I can
>>> more easily investigate the bug:
>>> > https://reviews.llvm.org/rL348604
>>> >
>>> >
>>> > On Fri, Dec 7, 2018 at 8:08 AM Sanjay Patel <spatel at rotateright.com>
>>> wrote:
>>> >>
>>> >> Thanks. Yes, I agree - now that we have a repro test and know it's
>>> not limited to PPC, we should revert. I'll do that shortly.
>>> >>
>>> >> On Fri, Dec 7, 2018 at 5:30 AM Hans Wennborg via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>> >>>
>>> >>> I just bisected an x86 Android Chrome build failure to this revision,
>>> >>> so it's not PPC specific.
>>> >>>
>>> >>> I've uploaded a repro here:
>>> >>> https://bugs.chromium.org/p/chromium/issues/detail?id=912878#c3
>>> >>>
>>> >>> Before this change, the test compiles in about 20 s on my machine.
>>> >>> After the change, it runs for several minutes and then seems to hit
>>> an
>>> >>> assert in SmallVector (due to out-of-memory I suppose).
>>> >>>
>>> >>> Since we now have a non-PPC test case that clearly shows this is a
>>> >>> regression, I think the change should be reverted until it's been
>>> >>> investigated.
>>> >>>
>>> >>> On Fri, Dec 7, 2018 at 2:01 AM Nemanja Ivanovic via llvm-commits
>>> >>> <llvm-commits at lists.llvm.org> wrote:
>>> >>> >
>>> >>> > I am sorry that I am jumping into this so late and I don't know
>>> how it is that I am missing some of the original context. At some point
>>> there were mentions of a PPC specific problem. However this doesn't seem to
>>> be the case at this point. If the group on this thread still feels that
>>> there is a PPC specific component to this, please let me know how to
>>> reproduce and I will gladly do the PPC investigation.
>>> >>> >
>>> >>> > On Dec 6, 2018 7:26 PM, "Craig Topper" <craig.topper at gmail.com>
>>> wrote:
>>> >>> >
>>> >>> > 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.)
>>> >>> >
>>> >>> >
>>> >>> > _______________________________________________
>>> >>> > 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
>>> _______________________________________________
>>> 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/20181208/dec5a3b7/attachment-0001.html>