[llvm] 077bff3 - [Analysis] isDereferenceableAndAlignedPointer(): recurse into select's hands
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 14 09:23:22 PDT 2021
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210414/7b1e0e55/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4002 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210414/7b1e0e55/attachment-0001.bin>
More information about the llvm-commits
mailing list