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

[Roman Lebedev via llvm-commits]

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?
Does this happen exactly with this patch, or with some later change?
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.

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