[llvm] 077bff3 - [Analysis] isDereferenceableAndAlignedPointer(): recurse into select's hands

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 14 10:00:14 PDT 2021


If there are no further reproducers i consider the issues here to be resolved.
Thanks.

On Wed, Apr 14, 2021 at 7:23 PM Jordan Rupprecht <rupprecht at google.com> wrote:
>
> The source repro for the assertion is one line different (initialize a & b to different values):
>
> #include <string_view>
> #include <unordered_map>
> #include <vector>
>
> void Func(std::string_view x) {
>   std::unordered_map<std::string_view, int> m;
>   constexpr std::string_view a = "a", b = "b";
>
>   std::vector<std::string_view> xs = {x == a ? a : b};
>   for (auto x_i : xs) {
>     m.find(x_i);
>   }
> }
>
> However, this repro requires the internal FDO prof file that I can't figure out how to reduce.
>
> While I was working on that, kramerb committed cf4161673c7e7c7c57d8115468bfcc9988f43d36 (looks like 2fea5d5d4accf3490854b064a51d1db049b1de64 was causing other clang crashes), and that fixes the assertion I was seeing too. So I guess the test case added there might work.
>
> On Wed, Apr 14, 2021 at 5:06 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>>
>> Ok, i've dealt with that verifier error in
>> 2fea5d5d4accf3490854b064a51d1db049b1de64.
>> That isn't a proper fix, but more like a band-aid that should be adjusted later,
>> because it is way too restrictive.
>>
>> Looking forward to seeing a reproducer for the second problem, the assertion.
>>
>> Roman
>>
>> On Wed, Apr 14, 2021 at 11:11 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>> >
>> > On Wed, Apr 14, 2021 at 7:08 AM Jordan Rupprecht <rupprecht at google.com> wrote:
>> > >
>> > > Not sure what I was doing wrong before, but here's the clang invocation that crashes (repro.cc already provided):
>> > >
>> > > $ clang \
>> > >   -fno-exceptions -fmerge-all-constants \
>> > >   -std=gnu++17 -stdlib=libc++ \
>> > >   -O1 \
>> > >   -c /tmp/repro.cc \
>> > >   -o /tmp/repro.o
>> > >
>> > > If you have suggestions on how to generate an IR reproducer, I'd like to know how -- I'm running "-O0 -Xclang -disable-O0-optnone -S -emit-llvm" to generate the IR, but that doesn't crash opt or clang when running those with >= -O1.
>> > Worksforme:
>> >
>> > $ ./bin/clang -fno-exceptions -fmerge-all-constants -std=gnu++17
>> > -stdlib=libc++ -O3 -c /tmp/test.cpp -S -emit-llvm
>> > -fdiscard-value-names -Xclang -disable-llvm-optzns -S -emit-llvm -o
>> > zz.l
>> > $ ./bin/opt -O1 zz.ll
>> > WARNING: You're attempting to print out a bitcode file.
>> > This is inadvisable as it may cause display problems. If
>> > you REALLY want to taste LLVM bitcode first-hand, you
>> > can force output with the `-f' option.
>> >
>> > Instruction does not dominate all uses!
>> >  %.not.i = icmp eq i64 %1, 0
>> >  %.sroa.016.0..sroa_cast20 = select i1 %.not.i, i8* bitcast
>> > (%"class.std::__1::basic_string_view"*
>> > @_ZZ4FuncNSt3__117basic_string_viewIcNS_11char_traitsIcEEEEE1b to
>> > i8*), i8* bitcast (%"class.std::__1::basic_string_view"*
>> > @_ZZ4FuncNSt3__117basic_string_viewIcNS_11char_
>> > traitsIcEEEEE1b to i8*)
>> > LLVM ERROR: Broken module found, compilation aborted!
>> > PLEASE submit a bug report to https://bugs.llvm.org/ and include the
>> > crash backtrace.
>> > Stack dump:
>> > 0.      Program arguments: ./bin/opt -O1 zz.ll
>> > #0 0x00007f02b1503e43 llvm::sys::PrintStackTrace(llvm::raw_ostream&,
>> > int) /repositories/llvm-project/llvm/lib/Support/Unix/Signals.inc:565:13
>> > #1 0x00007f02b15019a0 llvm::sys::RunSignalHandlers()
>> > /repositories/llvm-project/llvm/lib/Support/Signals.cpp:77:18
>> > #2 0x00007f02b150435f SignalHandler(int)
>> > /repositories/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
>> > #3 0x00007f02b4587140 __restore_rt
>> > (/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
>> > #4 0x00007f02b0e11ce1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
>> > #5 0x00007f02b0dfb537 abort ./stdlib/abort.c:81:7
>> > #6 0x00007f02b142c3bb llvm::SmallVectorImpl<char>::~SmallVectorImpl()
>> > /repositories/llvm-project/llvm/include/llvm/ADT/SmallVector.h:582:7
>> > #7 0x00007f02b142c3bb llvm::SmallVector<char, 64u>::~SmallVector()
>> > /repositories/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1176:3
>> > #8 0x00007f02b142c3bb llvm::report_fatal_error(llvm::Twine const&,
>> > bool) /repositories/llvm-project/llvm/lib/Support/ErrorHandling.cpp:119:3
>> > #9 0x00007f02b142c1f8
>> > (/builddirs/llvm-project/build-Clang11/bin/../lib/libLLVMSupport.so.13git+0x1661f8)
>> > #10 0x00007f02b19fb156 llvm::VerifierPass::run(llvm::Module&,
>> > llvm::AnalysisManager<llvm::Module>&)
>> > /repositories/llvm-project/llvm/lib/IR/Verifier.cpp:0:5
>> > #11 0x000000000024186d llvm::detail::PassModel<llvm::Module,
>> > llvm::VerifierPass, llvm::PreservedAnalyses,
>> > llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&,
>> > llvm::AnalysisManager<llvm::Module>&)
>> > /repositories/llvm-project/llvm/include/llvm/IR/PassManagerInternal.
>> > h:85:5
>> > #12 0x00007f02b19c3ac8 llvm::SmallPtrSet<void*,
>> > 2u>::operator=(llvm::SmallPtrSet<void*, 2u>&&)
>> > /repositories/llvm-project/llvm/include/llvm/ADT/SmallPtrSet.h:488:13
>> > #13 0x00007f02b19c3ac8
>> > llvm::PreservedAnalyses::operator=(llvm::PreservedAnalyses&&)
>> > /repositories/llvm-project/llvm/include/llvm/IR/PassManager.h:155:7
>> > #14 0x00007f02b19c3ac8 llvm::PassManager<llvm::Module,
>> > llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&,
>> > llvm::AnalysisManager<llvm::Module>&)
>> > /repositories/llvm-project/llvm/include/llvm/IR/PassManager.h:517:16
>> > #15 0x00000000002378d6 llvm::SmallPtrSetImplBase::isSmall() const
>> > /repositories/llvm-project/llvm/include/llvm/ADT/SmallPtrSet.h:194:33
>> > #16 0x00000000002378d6
>> > llvm::SmallPtrSetImplBase::~SmallPtrSetImplBase()
>> > /repositories/llvm-project/llvm/include/llvm/ADT/SmallPtrSet.h:82:10
>> > #17 0x00000000002378d6 llvm::PreservedAnalyses::~PreservedAnalyses()
>> > /repositories/llvm-project/llvm/include/llvm/IR/PassManager.h:155:7
>> > #18 0x00000000002378d6 llvm::runPassPipeline(llvm::StringRef,
>> > llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*,
>> > llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*,
>> > llvm::StringRef, llvm::ArrayRef<llvm::StringRef>,
>> > llvm::opt_tool::Output
>> > Kind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool,
>> > bool) /repositories/llvm-project/llvm/tools/opt/NewPMDriver.cpp:444:3
>> > #19 0x000000000024eeab main
>> > /repositories/llvm-project/llvm/tools/opt/opt.cpp:827:12
>> > #20 0x00007f02b0dfcd0a __libc_start_main ./csu/../csu/libc-start.c:308:16
>> > #21 0x000000000022fc6a _start (./bin/opt+0x22fc6a)
>> > Aborted
>> >
>> > > On Tue, Apr 13, 2021 at 8:17 PM Jordan Rupprecht <rupprecht at google.com> wrote:
>> > >>
>> > >> I managed to reduce the C++ source reproducer to:
>> > >>
>> > >> ```
>> > >> #include <string_view>
>> > >> #include <unordered_map>
>> > >> #include <vector>
>> > >>
>> > >> void Func(std::string_view x) {
>> > >>   std::unordered_map<std::string_view, int> m;
>> > >>   constexpr std::string_view a = "", b = "";
>> > >>
>> > >>   std::vector<std::string_view> xs = {x == a ? a : b};
>> > >>   for (auto x_i : xs) {
>> > >>     m.find(x_i);
>> > >>   }
>> > >> }
>> > >> ```
>> > >> =>
>> > >> ```
>> > >> Instruction does not dominate all uses!
>> > >>   %12 = icmp eq i64 %1, 0
>> > >>   %11 = select i1 %12, i8* bitcast (%"class.std::__u::basic_string_view"* @_ZZ4FuncNSt3__u17basic_string_viewIcNS_11char_traitsIcEEEEE1a to i8*), i8* bitcast (%"class.std::__u::basic_string_view"* @_ZZ4FuncNSt3__u17basic_string_viewIcNS_11char_traitsIcEEEEE1b to i8*)
>> > >> ```
>> > >>
>> > >> Unfortunately, getting this to print the "Instruction does not dominate all uses!" error still requires a ton of flags, including an FDO profile from internal sources. Haven't gotten an IR repro out of this yet.
>> > >> (This reduction does not trigger the assertion in debug mode anymore, btw)
>> > >>
>> > >> On Tue, Apr 13, 2021 at 3:01 PM Jordan Rupprecht <rupprecht at google.com> wrote:
>> > >>>
>> > >>>
>> > >>>
>> > >>> On Tue, Apr 13, 2021 at 2:06 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>> > >>>>
>> > >>>> On Tue, Apr 13, 2021 at 9:56 PM Jordan Rupprecht <rupprecht at google.com> wrote:
>> > >>>> >
>> > >>>> > We're seeing a clang crash (Instruction does not dominate all uses) as a result of this patch. With asserts enabled, I see:
>> > >>>> >
>> > >>>> > assert.h assertion failed at llvm/lib/Analysis/LazyValueInfo.cpp:677 in Optional<llvm::ValueLatticeElement> (anonymous namespace)::LazyValueInfoImpl::solveBlockValueNonLocal(llvm::Value *, llvm::BasicBlock *): isa<Argument>(Val) && "Unknown live-in to the entry block"
>> > >>>> >
>> > >>>> > Mind if we revert this patch while we come up with a reduction? And/or is the error message above enough to clue you on what the right fix is?
>> > >>>> >
>> > >>>> > The reproduction requires FDO, so reduction is trickier :( but I'll get started
>> > >>>> Which pass crashes? CVP?
>> > >>>
>> > >>>
>> > >>> The crash we see with release builds doesn't have a stack trace, so I'm not sure which pass it is. The full error is:
>> > >>> """
>> > >>> Instruction does not dominate all uses!
>> > >>>   %177 = phi i1 [ %175, %173 ], [ false, %165 ]
>> > >>>   %170 = select i1 %177, i8* bitcast (%"class.std::__u::basic_string_view"* @<foo> to i8*), i8* bitcast (%"class.std::__u::basic_string_view"* @<bar> to i8*)
>> > >>> fatal error: error in backend: Broken module found, compilation aborted!
>> > >>> clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
>> > >>> """
>> > >>> The assertion failure we see in a debug-built clang happens in llvm::CorrelatedValuePropagationPass::run
>> > >>>
>> > >>>>
>> > >>>> Does this happen exactly with this patch, or with some later change?
>> > >>>
>> > >>> The "Instruction does not dominate all uses!" failure in release builds bisects to this patch. I'm having some difficulties with bisecting the assertion failure (because of the FDO issue), but it doesn't happen in a weeks-old compiler. So either it bisects to the same thing, or something else that's very new. I can keep looking at that.
>> > >>>
>> > >>>>
>> > >>>> What if you dump full module IR before the pass, and run just that
>> > >>>> single pass via `opt`, does it crash?
>> > >>>>
>> > >>>> Unless the assertion is triggered because the [base?] compiler was miscompiled,
>> > >>>> i think that assertion is simply over-eager and should be removed.
>> > >>>> But i won't be able to tell without the testcase.
>> > >>>
>> > >>> (still working on a test case)
>> > >>>
>> > >>>>
>> > >>>>
>> > >>>> Roman
>> > >>>>
>> > >>>> > On Fri, Apr 9, 2021 at 4:56 PM Roman Lebedev via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> > >>>> >>
>> > >>>> >>
>> > >>>> >> Author: Roman Lebedev
>> > >>>> >> Date: 2021-04-10T00:56:28+03:00
>> > >>>> >> New Revision: 077bff39d46364035a5dcfa32fc69910ad0975d0
>> > >>>> >>
>> > >>>> >> URL: https://github.com/llvm/llvm-project/commit/077bff39d46364035a5dcfa32fc69910ad0975d0
>> > >>>> >> DIFF: https://github.com/llvm/llvm-project/commit/077bff39d46364035a5dcfa32fc69910ad0975d0.diff
>> > >>>> >>
>> > >>>> >> LOG: [Analysis] isDereferenceableAndAlignedPointer(): recurse into select's hands
>> > >>>> >>
>> > >>>> >> By doing this within the method itself,
>> > >>>> >> we support traversing multiple levels of selects (TODO: PHI's),
>> > >>>> >> fixing the SROA `std::clamp()` testcase.
>> > >>>> >>
>> > >>>> >> Fixes https://bugs.llvm.org/show_bug.cgi?id=47271
>> > >>>> >> Mostly fixes https://bugs.llvm.org/show_bug.cgi?id=49909
>> > >>>> >>
>> > >>>> >> Added:
>> > >>>> >>
>> > >>>> >>
>> > >>>> >> Modified:
>> > >>>> >>     llvm/lib/Analysis/Loads.cpp
>> > >>>> >>     llvm/test/Transforms/SROA/std-clamp.ll
>> > >>>> >>
>> > >>>> >> Removed:
>> > >>>> >>
>> > >>>> >>
>> > >>>> >>
>> > >>>> >> ################################################################################
>> > >>>> >> diff  --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
>> > >>>> >> index 8d01ea045097..2ae1fd5b5bde 100644
>> > >>>> >> --- a/llvm/lib/Analysis/Loads.cpp
>> > >>>> >> +++ b/llvm/lib/Analysis/Loads.cpp
>> > >>>> >> @@ -59,6 +59,16 @@ static bool isDereferenceableAndAlignedPointer(
>> > >>>> >>    // Note that it is not safe to speculate into a malloc'd region because
>> > >>>> >>    // malloc may return null.
>> > >>>> >>
>> > >>>> >> +  // Recurse into both hands of select.
>> > >>>> >> +  if (const SelectInst *Sel = dyn_cast<SelectInst>(V)) {
>> > >>>> >> +    return isDereferenceableAndAlignedPointer(Sel->getTrueValue(), Alignment,
>> > >>>> >> +                                              Size, DL, CtxI, DT, TLI, Visited,
>> > >>>> >> +                                              MaxDepth) &&
>> > >>>> >> +           isDereferenceableAndAlignedPointer(Sel->getFalseValue(), Alignment,
>> > >>>> >> +                                              Size, DL, CtxI, DT, TLI, Visited,
>> > >>>> >> +                                              MaxDepth);
>> > >>>> >> +  }
>> > >>>> >> +
>> > >>>> >>    // bitcast instructions are no-ops as far as dereferenceability is concerned.
>> > >>>> >>    if (const BitCastOperator *BC = dyn_cast<BitCastOperator>(V)) {
>> > >>>> >>      if (BC->getSrcTy()->isPointerTy())
>> > >>>> >>
>> > >>>> >> diff  --git a/llvm/test/Transforms/SROA/std-clamp.ll b/llvm/test/Transforms/SROA/std-clamp.ll
>> > >>>> >> index 561904252288..c0132a519722 100644
>> > >>>> >> --- a/llvm/test/Transforms/SROA/std-clamp.ll
>> > >>>> >> +++ b/llvm/test/Transforms/SROA/std-clamp.ll
>> > >>>> >> @@ -6,18 +6,14 @@ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
>> > >>>> >>  define float @_Z8stdclampfff(float %x, float %lo, float %hi) {
>> > >>>> >>  ; CHECK-LABEL: @_Z8stdclampfff(
>> > >>>> >>  ; CHECK-NEXT:  bb:
>> > >>>> >> -; CHECK-NEXT:    [[I:%.*]] = alloca float, align 4
>> > >>>> >> -; CHECK-NEXT:    [[I3:%.*]] = alloca float, align 4
>> > >>>> >>  ; CHECK-NEXT:    [[I4:%.*]] = alloca float, align 4
>> > >>>> >> -; CHECK-NEXT:    store float [[X:%.*]], float* [[I]], align 4
>> > >>>> >> -; CHECK-NEXT:    store float [[LO:%.*]], float* [[I3]], align 4
>> > >>>> >>  ; CHECK-NEXT:    store float [[HI:%.*]], float* [[I4]], align 4
>> > >>>> >> -; CHECK-NEXT:    [[I5:%.*]] = fcmp fast olt float [[X]], [[LO]]
>> > >>>> >> +; CHECK-NEXT:    [[I5:%.*]] = fcmp fast olt float [[X:%.*]], [[LO:%.*]]
>> > >>>> >>  ; CHECK-NEXT:    [[I6:%.*]] = fcmp fast olt float [[HI]], [[X]]
>> > >>>> >> -; CHECK-NEXT:    [[I7:%.*]] = select i1 [[I6]], float* [[I4]], float* [[I]]
>> > >>>> >> -; CHECK-NEXT:    [[I8:%.*]] = select i1 [[I5]], float* [[I3]], float* [[I7]]
>> > >>>> >> -; CHECK-NEXT:    [[I9:%.*]] = load float, float* [[I8]], align 4
>> > >>>> >> -; CHECK-NEXT:    ret float [[I9]]
>> > >>>> >> +; CHECK-NEXT:    [[I9_SROA_SPECULATE_LOAD_FALSE_SROA_SPECULATE_LOAD_TRUE:%.*]] = load float, float* [[I4]], align 4
>> > >>>> >> +; CHECK-NEXT:    [[I9_SROA_SPECULATE_LOAD_FALSE_SROA_SPECULATED:%.*]] = select i1 [[I6]], float [[I9_SROA_SPECULATE_LOAD_FALSE_SROA_SPECULATE_LOAD_TRUE]], float [[X]]
>> > >>>> >> +; CHECK-NEXT:    [[I9_SROA_SPECULATED:%.*]] = select i1 [[I5]], float [[LO]], float [[I9_SROA_SPECULATE_LOAD_FALSE_SROA_SPECULATED]]
>> > >>>> >> +; CHECK-NEXT:    ret float [[I9_SROA_SPECULATED]]
>> > >>>> >>  ;
>> > >>>> >>  bb:
>> > >>>> >>    %i = alloca float, align 4
>> > >>>> >>
>> > >>>> >>
>> > >>>> >>
>> > >>>> >> _______________________________________________
>> > >>>> >> llvm-commits mailing list
>> > >>>> >> llvm-commits at lists.llvm.org
>> > >>>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list