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

Tue Apr 13 13:01:06 PDT 2021

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/3aab9ff2/attachment.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/3aab9ff2/attachment.bin>