From 87650a2cac7b176885eff9577932546e3bf6fb3f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 8 May 2019 14:50:01 +0000 Subject: [PATCH] Revert "[ValueTracking] Improve isKnowNonZero for Ints" This reverts commit 3b137a495686bd6018d115ea82fb8bb7718349fd. As reported in https://reviews.llvm.org/D60846, this is causing miscompiles. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360260 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ValueTracking.cpp | 26 +++++++++++++------------- test/Transforms/InstCombine/known-non-zero.ll | 4 ++-- test/Transforms/InstSimplify/known-non-zero.ll | 10 +++++++--- test/Transforms/LICM/hoist-mustexec.ll | 4 +++- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index 839f539707a..54575441b71 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -1893,9 +1893,10 @@ static bool isGEPKnownNonNull(const GEPOperator *GEP, unsigned Depth, return false; } -static bool isKnownNonZeroFromDominatingCondition(const Value *V, +static bool isKnownNonNullFromDominatingCondition(const Value *V, const Instruction *CtxI, const DominatorTree *DT) { + assert(V->getType()->isPointerTy() && "V must be pointer type"); assert(!isa(V) && "Did not expect ConstantPointerNull"); if (!CtxI || !DT) @@ -1908,15 +1909,14 @@ static bool isKnownNonZeroFromDominatingCondition(const Value *V, break; NumUsesExplored++; - // If the value is a pointer and used as an argument to a call or invoke, - // then argument attributes may provide an answer about null-ness. - if (V->getType()->isPointerTy()) - if (auto CS = ImmutableCallSite(U)) - if (auto *CalledFunc = CS.getCalledFunction()) - for (const Argument &Arg : CalledFunc->args()) - if (CS.getArgOperand(Arg.getArgNo()) == V && - Arg.hasNonNullAttr() && DT->dominates(CS.getInstruction(), CtxI)) - return true; + // If the value is used as an argument to a call or invoke, then argument + // attributes may provide an answer about null-ness. + if (auto CS = ImmutableCallSite(U)) + if (auto *CalledFunc = CS.getCalledFunction()) + for (const Argument &Arg : CalledFunc->args()) + if (CS.getArgOperand(Arg.getArgNo()) == V && + Arg.hasNonNullAttr() && DT->dominates(CS.getInstruction(), CtxI)) + return true; // Consider only compare instructions uniquely controlling a branch CmpInst::Predicate Pred; @@ -2064,11 +2064,11 @@ bool isKnownNonZero(const Value *V, unsigned Depth, const Query &Q) { } - if (isKnownNonZeroFromDominatingCondition(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/test/Transforms/InstCombine/known-non-zero.ll b/test/Transforms/InstCombine/known-non-zero.ll index d587bfa634b..0329fbaf990 100644 --- a/test/Transforms/InstCombine/known-non-zero.ll +++ b/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 true), !range !0 +; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.cttz.i64(i64 [[X]], i1 false), !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 true), !range !0 +; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.ctlz.i64(i64 [[X]], i1 false), !range !0 ; CHECK-NEXT: [[CTZ32:%.*]] = trunc i64 [[CTZ]] to i32 ; CHECK-NEXT: br label [[EXIT]] ; CHECK: exit: diff --git a/test/Transforms/InstSimplify/known-non-zero.ll b/test/Transforms/InstSimplify/known-non-zero.ll index 1a985ede9a4..7e819e82b90 100644 --- a/test/Transforms/InstSimplify/known-non-zero.ll +++ b/test/Transforms/InstSimplify/known-non-zero.ll @@ -7,7 +7,8 @@ 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: br i1 false, label [[UNREACHABLE:%.*]], label [[EXIT]] +; CHECK-NEXT: [[B:%.*]] = icmp eq i64 [[X]], 0 +; CHECK-NEXT: br i1 [[B]], label [[UNREACHABLE:%.*]], label [[EXIT]] ; CHECK: unreachable: ; CHECK-NEXT: br label [[EXIT]] ; CHECK: exit: @@ -36,7 +37,8 @@ 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: br i1 true, label [[EXIT]], label [[UNREACHABLE:%.*]] +; CHECK-NEXT: [[B:%.*]] = icmp ugt i64 [[X]], 0 +; CHECK-NEXT: br i1 [[B]], label [[EXIT]], label [[UNREACHABLE:%.*]] ; CHECK: unreachable: ; CHECK-NEXT: br label [[EXIT]] ; CHECK: exit: @@ -71,9 +73,11 @@ 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 [ false, [[MAINBLOCK]] ], [ true, [[START:%.*]] ] +; CHECK-NEXT: [[RES:%.*]] = phi i1 [ [[CMP]], [[MAINBLOCK]] ], [ true, [[START:%.*]] ] ; CHECK-NEXT: ret i1 [[RES]] ; start: diff --git a/test/Transforms/LICM/hoist-mustexec.ll b/test/Transforms/LICM/hoist-mustexec.ll index 59184eb542c..521d3529657 100644 --- a/test/Transforms/LICM/hoist-mustexec.ll +++ b/test/Transforms/LICM/hoist-mustexec.ll @@ -129,6 +129,8 @@ 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: @@ -136,7 +138,6 @@ 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:%.*]] ] @@ -144,6 +145,7 @@ 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 -- 2.11.0