[llvm] 077bff3 - [Analysis] isDereferenceableAndAlignedPointer(): recurse into select's hands
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 13 21:08:06 PDT 2021
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.
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/20210413/caf017a1/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/20210413/caf017a1/attachment-0001.bin>
More information about the llvm-commits
mailing list