[llvm] 57dd4b0 - [ValueTracking] Allow context-sensitive nullness check for non-pointers
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 03:23:49 PST 2019
I've reverted this in 6ea4775 as it miscompiled Chromium. Sorry for
not catching it earlier, there's been a lot of breakages lately.
I included a nice and short reproducer in the revert commit message.
On Thu, Oct 31, 2019 at 8:38 PM Johannes Doerfert via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Johannes Doerfert
> Date: 2019-10-31T14:37:38-05:00
> New Revision: 57dd4b03e4806bbb4760ab6150940150d884df20
>
> URL: https://github.com/llvm/llvm-project/commit/57dd4b03e4806bbb4760ab6150940150d884df20
> DIFF: https://github.com/llvm/llvm-project/commit/57dd4b03e4806bbb4760ab6150940150d884df20.diff
>
> LOG: [ValueTracking] Allow context-sensitive nullness check for non-pointers
>
> Same as D60846 but with a fix for the problem encountered there which
> was a missing context adjustment in the handling of PHI nodes.
>
> The test that caused D60846 to be reverted was added in e15ab8f277c7.
>
> Reviewers: nikic, nlopes, mkazantsev,spatel, dlrobertson, uabelho, hakzsam
>
> Subscribers: hiraditya, bollu, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D69571
>
> Added:
>
>
> Modified:
> llvm/lib/Analysis/InstructionSimplify.cpp
> llvm/lib/Analysis/ValueTracking.cpp
> llvm/test/Transforms/InstCombine/known-non-zero.ll
> llvm/test/Transforms/InstSimplify/known-non-zero.ll
> llvm/test/Transforms/LICM/hoist-mustexec.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
> index cb8987721700..11a12ab84e9b 100644
> --- a/llvm/lib/Analysis/InstructionSimplify.cpp
> +++ b/llvm/lib/Analysis/InstructionSimplify.cpp
> @@ -543,10 +543,16 @@ static Value *ThreadCmpOverPHI(CmpInst::Predicate Pred, Value *LHS, Value *RHS,
>
> // Evaluate the BinOp on the incoming phi values.
> Value *CommonValue = nullptr;
> - for (Value *Incoming : PI->incoming_values()) {
> + for (unsigned u = 0, e = PI->getNumIncomingValues(); u < e; ++u) {
> + Value *Incoming = PI->getIncomingValue(u);
> + Instruction *InTI = PI->getIncomingBlock(u)->getTerminator();
> // If the incoming value is the phi node itself, it can safely be skipped.
> if (Incoming == PI) continue;
> - Value *V = SimplifyCmpInst(Pred, Incoming, RHS, Q, MaxRecurse);
> + // Change the context instruction to the "edge" that flows into the phi.
> + // This is important because that is where incoming is actually "evaluated"
> + // even though it is used later somewhere else.
> + Value *V = SimplifyCmpInst(Pred, Incoming, RHS, Q.getWithInstruction(InTI),
> + MaxRecurse);
> // If the operation failed to simplify, or simplified to a
> diff erent value
> // to previously, then give up.
> if (!V || (CommonValue && V != CommonValue))
>
> diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
> index bbf389991836..cc5acf5d010a 100644
> --- a/llvm/lib/Analysis/ValueTracking.cpp
> +++ b/llvm/lib/Analysis/ValueTracking.cpp
> @@ -1902,8 +1902,8 @@ static bool isGEPKnownNonNull(const GEPOperator *GEP, unsigned Depth,
> static bool isKnownNonNullFromDominatingCondition(const Value *V,
> const Instruction *CtxI,
> const DominatorTree *DT) {
> - assert(V->getType()->isPointerTy() && "V must be pointer type");
> - assert(!isa<ConstantData>(V) && "Did not expect ConstantPointerNull");
> + if (isa<Constant>(V))
> + return false;
>
> if (!CtxI || !DT)
> return false;
> @@ -2078,12 +2078,11 @@ bool isKnownNonZero(const Value *V, unsigned Depth, const Query &Q) {
> }
> }
>
> + if (isKnownNonNullFromDominatingCondition(V, Q.CxtI, Q.DT))
> + return true;
>
> // Check for recursive pointer simplifications.
> if (V->getType()->isPointerTy()) {
> - if (isKnownNonNullFromDominatingCondition(V, Q.CxtI, Q.DT))
> - return true;
> -
> // Look through bitcast operations, GEPs, and int2ptr instructions as they
> // do not alter the value, or at least not the nullness property of the
> // value, e.g., int2ptr is allowed to zero/sign extend the value.
>
> diff --git a/llvm/test/Transforms/InstCombine/known-non-zero.ll b/llvm/test/Transforms/InstCombine/known-non-zero.ll
> index 57980e0890c2..5467db556632 100644
> --- a/llvm/test/Transforms/InstCombine/known-non-zero.ll
> +++ b/llvm/test/Transforms/InstCombine/known-non-zero.ll
> @@ -13,7 +13,7 @@ define i32 @test0(i64 %x) {
> ; CHECK-NEXT: [[C:%.*]] = icmp eq i64 [[X:%.*]], 0
> ; CHECK-NEXT: br i1 [[C]], label [[EXIT:%.*]], label [[NON_ZERO:%.*]]
> ; CHECK: non_zero:
> -; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.cttz.i64(i64 [[X]], i1 false), !range !0
> +; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.cttz.i64(i64 [[X]], i1 true), !range !0
> ; CHECK-NEXT: [[CTZ32:%.*]] = trunc i64 [[CTZ]] to i32
> ; CHECK-NEXT: br label [[EXIT]]
> ; CHECK: exit:
> @@ -40,7 +40,7 @@ define i32 @test1(i64 %x) {
> ; CHECK-NEXT: [[C:%.*]] = icmp eq i64 [[X:%.*]], 0
> ; CHECK-NEXT: br i1 [[C]], label [[EXIT:%.*]], label [[NON_ZERO:%.*]]
> ; CHECK: non_zero:
> -; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.ctlz.i64(i64 [[X]], i1 false), !range !0
> +; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.ctlz.i64(i64 [[X]], i1 true), !range !0
> ; CHECK-NEXT: [[CTZ32:%.*]] = trunc i64 [[CTZ]] to i32
> ; CHECK-NEXT: br label [[EXIT]]
> ; CHECK: exit:
>
> diff --git a/llvm/test/Transforms/InstSimplify/known-non-zero.ll b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
> index 7e819e82b907..1a985ede9a43 100644
> --- a/llvm/test/Transforms/InstSimplify/known-non-zero.ll
> +++ b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
> @@ -7,8 +7,7 @@ define i64 @test0(i64 %x) {
> ; CHECK-NEXT: [[A:%.*]] = icmp eq i64 [[X:%.*]], 0
> ; CHECK-NEXT: br i1 [[A]], label [[EXIT:%.*]], label [[NON_ZERO:%.*]]
> ; CHECK: non_zero:
> -; CHECK-NEXT: [[B:%.*]] = icmp eq i64 [[X]], 0
> -; CHECK-NEXT: br i1 [[B]], label [[UNREACHABLE:%.*]], label [[EXIT]]
> +; CHECK-NEXT: br i1 false, label [[UNREACHABLE:%.*]], label [[EXIT]]
> ; CHECK: unreachable:
> ; CHECK-NEXT: br label [[EXIT]]
> ; CHECK: exit:
> @@ -37,8 +36,7 @@ define i64 @test1(i64 %x) {
> ; CHECK-NEXT: [[A:%.*]] = icmp eq i64 [[X:%.*]], 0
> ; CHECK-NEXT: br i1 [[A]], label [[EXIT:%.*]], label [[NON_ZERO:%.*]]
> ; CHECK: non_zero:
> -; CHECK-NEXT: [[B:%.*]] = icmp ugt i64 [[X]], 0
> -; CHECK-NEXT: br i1 [[B]], label [[EXIT]], label [[UNREACHABLE:%.*]]
> +; CHECK-NEXT: br i1 true, label [[EXIT]], label [[UNREACHABLE:%.*]]
> ; CHECK: unreachable:
> ; CHECK-NEXT: br label [[EXIT]]
> ; CHECK: exit:
> @@ -73,11 +71,9 @@ define i1 @test2(i64 %x, i1 %y) {
> ; CHECK: two:
> ; CHECK-NEXT: br label [[MAINBLOCK]]
> ; CHECK: mainblock:
> -; CHECK-NEXT: [[P:%.*]] = phi i64 [ [[X]], [[ONE]] ], [ 42, [[TWO]] ]
> -; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[P]], 0
> ; CHECK-NEXT: br label [[EXIT]]
> ; CHECK: exit:
> -; CHECK-NEXT: [[RES:%.*]] = phi i1 [ [[CMP]], [[MAINBLOCK]] ], [ true, [[START:%.*]] ]
> +; CHECK-NEXT: [[RES:%.*]] = phi i1 [ false, [[MAINBLOCK]] ], [ true, [[START:%.*]] ]
> ; CHECK-NEXT: ret i1 [[RES]]
> ;
> start:
>
> diff --git a/llvm/test/Transforms/LICM/hoist-mustexec.ll b/llvm/test/Transforms/LICM/hoist-mustexec.ll
> index 521d35296572..59184eb542cf 100644
> --- a/llvm/test/Transforms/LICM/hoist-mustexec.ll
> +++ b/llvm/test/Transforms/LICM/hoist-mustexec.ll
> @@ -129,8 +129,6 @@ fail:
> }
>
> ; requires fact length is non-zero
> -; TODO: IsKnownNonNullFromDominatingConditions is currently only be done for
> -; pointers; should handle integers too
> define i32 @test4(i32* noalias nocapture readonly %a) nounwind uwtable {
> ; CHECK-LABEL: @test4(
> ; CHECK-NEXT: entry:
> @@ -138,6 +136,7 @@ define i32 @test4(i32* noalias nocapture readonly %a) nounwind uwtable {
> ; CHECK-NEXT: [[IS_ZERO:%.*]] = icmp eq i32 [[LEN]], 0
> ; CHECK-NEXT: br i1 [[IS_ZERO]], label [[FAIL:%.*]], label [[PREHEADER:%.*]]
> ; CHECK: preheader:
> +; CHECK-NEXT: [[I1:%.*]] = load i32, i32* [[A]], align 4
> ; CHECK-NEXT: br label [[FOR_BODY:%.*]]
> ; CHECK: for.body:
> ; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, [[PREHEADER]] ], [ [[INC:%.*]], [[CONTINUE:%.*]] ]
> @@ -145,7 +144,6 @@ define i32 @test4(i32* noalias nocapture readonly %a) nounwind uwtable {
> ; CHECK-NEXT: [[R_CHK:%.*]] = icmp ult i32 [[IV]], [[LEN]]
> ; CHECK-NEXT: br i1 [[R_CHK]], label [[CONTINUE]], label [[FAIL_LOOPEXIT:%.*]]
> ; CHECK: continue:
> -; CHECK-NEXT: [[I1:%.*]] = load i32, i32* [[A]], align 4
> ; CHECK-NEXT: [[ADD]] = add nsw i32 [[I1]], [[ACC]]
> ; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[IV]], 1
> ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[INC]], 1000
>
>
>
> _______________________________________________
> 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