diff --git a/llvm/include/llvm/Analysis/Loads.h b/llvm/include/llvm/Analysis/Loads.h index 0926093bba99d..a8d954b9872d9 100644 --- a/llvm/include/llvm/Analysis/Loads.h +++ b/llvm/include/llvm/Analysis/Loads.h @@ -173,14 +173,17 @@ Value *findAvailablePtrLoadStore(const MemoryLocation &Loc, Type *AccessTy, unsigned MaxInstsToScan, BatchAAResults *AA, bool *IsLoadCSE, unsigned *NumScanedInst); -/// Returns true if a pointer value \p A can be replace with another pointer -/// value \B if they are deemed equal through some means (e.g. information from +/// Returns true if a pointer value \p From can be replaced with another pointer +/// value \To if they are deemed equal through some means (e.g. information from /// conditions). -/// NOTE: the current implementations is incomplete and unsound. It does not -/// reject all invalid cases yet, but will be made stricter in the future. In -/// particular this means returning true means unknown if replacement is safe. -bool canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL, - Instruction *CtxI); +/// NOTE: The current implementation allows replacement in Icmp and PtrToInt +/// instructions, as well as when we are replacing with a null pointer. +/// Additionally it also allows replacement of pointers when both pointers have +/// the same underlying object. +bool canReplacePointersIfEqual(const Value *From, const Value *To, + const DataLayout &DL); +bool canReplacePointersInUseIfEqual(const Use &U, const Value *To, + const DataLayout &DL); } #endif diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index 9ae026fa95d21..e2143b5bfbe2f 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -439,6 +439,18 @@ unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT, /// the end of the given BasicBlock. Returns the number of replacements made. unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT, const BasicBlock *BB); +/// Replace each use of 'From' with 'To' if that use is dominated by +/// the given edge and the callback ShouldReplace returns true. Returns the +/// number of replacements made. +unsigned replaceDominatedUsesWithIf( + Value *From, Value *To, DominatorTree &DT, const BasicBlockEdge &Edge, + function_ref ShouldReplace); +/// Replace each use of 'From' with 'To' if that use is dominated by +/// the end of the given BasicBlock and the callback ShouldReplace returns true. +/// Returns the number of replacements made. +unsigned replaceDominatedUsesWithIf( + Value *From, Value *To, DominatorTree &DT, const BasicBlock *BB, + function_ref ShouldReplace); /// Return true if this call calls a gc leaf function. /// diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp index ac508e19c9e01..478302d687b53 100644 --- a/llvm/lib/Analysis/Loads.cpp +++ b/llvm/lib/Analysis/Loads.cpp @@ -710,22 +710,62 @@ Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BatchAAResults &AA, return Available; } -bool llvm::canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL, - Instruction *CtxI) { - Type *Ty = A->getType(); - assert(Ty == B->getType() && Ty->isPointerTy() && - "values must have matching pointer types"); - - // NOTE: The checks in the function are incomplete and currently miss illegal - // cases! The current implementation is a starting point and the - // implementation should be made stricter over time. - if (auto *C = dyn_cast(B)) { - // Do not allow replacing a pointer with a constant pointer, unless it is - // either null or at least one byte is dereferenceable. - APInt OneByte(DL.getPointerTypeSizeInBits(Ty), 1); - return C->isNullValue() || - isDereferenceableAndAlignedPointer(B, Align(1), OneByte, DL, CtxI); +// Returns true if a use is either in an ICmp/PtrToInt or a Phi/Select that only +// feeds into them. +static bool isPointerUseReplacable(const Use &U) { + unsigned Limit = 40; + SmallVector Worklist({U.getUser()}); + SmallPtrSet Visited; + + while (!Worklist.empty() && --Limit) { + auto *User = Worklist.pop_back_val(); + if (!Visited.insert(User).second) + continue; + if (isa(User)) + continue; + if (isa(User)) + Worklist.append(User->user_begin(), User->user_end()); + else + return false; } - return true; + return Limit != 0; +} + +// Returns true if `To` is a null pointer, constant dereferenceable pointer or +// both pointers have the same underlying objects. +static bool isPointerAlwaysReplaceable(const Value *From, const Value *To, + const DataLayout &DL) { + // This is not strictly correct, but we do it for now to retain important + // optimizations. + if (isa(To)) + return true; + if (isa(To) && + isDereferenceablePointer(To, Type::getInt8Ty(To->getContext()), DL)) + return true; + if (getUnderlyingObject(From) == getUnderlyingObject(To)) + return true; + return false; +} + +bool llvm::canReplacePointersInUseIfEqual(const Use &U, const Value *To, + const DataLayout &DL) { + assert(U->getType() == To->getType() && "values must have matching types"); + // Not a pointer, just return true. + if (!To->getType()->isPointerTy()) + return true; + + if (isPointerAlwaysReplaceable(&*U, To, DL)) + return true; + return isPointerUseReplacable(U); +} + +bool llvm::canReplacePointersIfEqual(const Value *From, const Value *To, + const DataLayout &DL) { + assert(From->getType() == To->getType() && "values must have matching types"); + // Not a pointer, just return true. + if (!From->getType()->isPointerTy()) + return true; + + return isPointerAlwaysReplaceable(From, To, DL); } diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index 86d5c9909f3dc..d829e92b24440 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -33,6 +33,7 @@ #include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/InstructionPrecedenceTracking.h" #include "llvm/Analysis/InstructionSimplify.h" +#include "llvm/Analysis/Loads.h" #include "llvm/Analysis/LoopInfo.h" #include "llvm/Analysis/MemoryBuiltins.h" #include "llvm/Analysis/MemoryDependenceAnalysis.h" @@ -2419,6 +2420,10 @@ bool GVNPass::propagateEquality(Value *LHS, Value *RHS, if (isa(LHS) || (isa(LHS) && !isa(RHS))) std::swap(LHS, RHS); assert((isa(LHS) || isa(LHS)) && "Unexpected value!"); + const DataLayout &DL = + isa(LHS) + ? cast(LHS)->getParent()->getParent()->getDataLayout() + : cast(LHS)->getModule()->getDataLayout(); // If there is no obvious reason to prefer the left-hand side over the // right-hand side, ensure the longest lived term is on the right-hand side, @@ -2445,23 +2450,32 @@ bool GVNPass::propagateEquality(Value *LHS, Value *RHS, // using the leader table is about compiling faster, not optimizing better). // The leader table only tracks basic blocks, not edges. Only add to if we // have the simple case where the edge dominates the end. - if (RootDominatesEnd && !isa(RHS)) + if (RootDominatesEnd && !isa(RHS) && + canReplacePointersIfEqual(LHS, RHS, DL)) addToLeaderTable(LVN, RHS, Root.getEnd()); // Replace all occurrences of 'LHS' with 'RHS' everywhere in the scope. As // LHS always has at least one use that is not dominated by Root, this will // never do anything if LHS has only one use. if (!LHS->hasOneUse()) { + // Create a callback that captures the DL. + auto canReplacePointersCallBack = [&DL](const Use &U, const Value *To) { + return canReplacePointersInUseIfEqual(U, To, DL); + }; unsigned NumReplacements = DominatesByEdge - ? replaceDominatedUsesWith(LHS, RHS, *DT, Root) - : replaceDominatedUsesWith(LHS, RHS, *DT, Root.getStart()); - - Changed |= NumReplacements > 0; - NumGVNEqProp += NumReplacements; - // Cached information for anything that uses LHS will be invalid. - if (MD) - MD->invalidateCachedPointerInfo(LHS); + ? replaceDominatedUsesWithIf(LHS, RHS, *DT, Root, + canReplacePointersCallBack) + : replaceDominatedUsesWithIf(LHS, RHS, *DT, Root.getStart(), + canReplacePointersCallBack); + + if (NumReplacements > 0) { + Changed = true; + NumGVNEqProp += NumReplacements; + // Cached information for anything that uses LHS will be invalid. + if (MD) + MD->invalidateCachedPointerInfo(LHS); + } } // Now try to deduce additional equalities from this one. For example, if diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 0a7b7a6cee757..5f456092bf4e9 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -3444,15 +3444,15 @@ void llvm::patchReplacementInstruction(Instruction *I, Value *Repl) { combineMetadataForCSE(ReplInst, I, false); } -template +template static unsigned replaceDominatedUsesWith(Value *From, Value *To, const RootType &Root, - const DominatesFn &Dominates) { + const ShouldReplaceFn &ShouldReplace) { assert(From->getType() == To->getType()); unsigned Count = 0; for (Use &U : llvm::make_early_inc_range(From->uses())) { - if (!Dominates(Root, U)) + if (!ShouldReplace(Root, U)) continue; LLVM_DEBUG(dbgs() << "Replace dominated use of '"; From->printAsOperand(dbgs()); @@ -3496,6 +3496,26 @@ unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To, return ::replaceDominatedUsesWith(From, To, BB, Dominates); } +unsigned llvm::replaceDominatedUsesWithIf( + Value *From, Value *To, DominatorTree &DT, const BasicBlockEdge &Root, + function_ref ShouldReplace) { + auto DominatesAndShouldReplace = + [&DT, &ShouldReplace, To](const BasicBlockEdge &Root, const Use &U) { + return DT.dominates(Root, U) && ShouldReplace(U, To); + }; + return ::replaceDominatedUsesWith(From, To, Root, DominatesAndShouldReplace); +} + +unsigned llvm::replaceDominatedUsesWithIf( + Value *From, Value *To, DominatorTree &DT, const BasicBlock *BB, + function_ref ShouldReplace) { + auto DominatesAndShouldReplace = [&DT, &ShouldReplace, + To](const BasicBlock *BB, const Use &U) { + return DT.dominates(BB, U) && ShouldReplace(U, To); + }; + return ::replaceDominatedUsesWith(From, To, BB, DominatesAndShouldReplace); +} + bool llvm::callsGCLeafFunction(const CallBase *Call, const TargetLibraryInfo &TLI) { // Check if the function is specifically marked as a gc leaf function. diff --git a/llvm/test/Transforms/GVN/condprop.ll b/llvm/test/Transforms/GVN/condprop.ll index 6b1e4d1060109..6402a23157729 100644 --- a/llvm/test/Transforms/GVN/condprop.ll +++ b/llvm/test/Transforms/GVN/condprop.ll @@ -214,11 +214,11 @@ define void @test4(i1 %b, i32 %x) { ; CHECK-NEXT: br i1 [[B:%.*]], label [[SW:%.*]], label [[CASE3:%.*]] ; CHECK: sw: ; CHECK-NEXT: switch i32 [[X:%.*]], label [[DEFAULT:%.*]] [ -; CHECK-NEXT: i32 0, label [[CASE0:%.*]] -; CHECK-NEXT: i32 1, label [[CASE1:%.*]] -; CHECK-NEXT: i32 2, label [[CASE0]] -; CHECK-NEXT: i32 3, label [[CASE3]] -; CHECK-NEXT: i32 4, label [[DEFAULT]] +; CHECK-NEXT: i32 0, label [[CASE0:%.*]] +; CHECK-NEXT: i32 1, label [[CASE1:%.*]] +; CHECK-NEXT: i32 2, label [[CASE0]] +; CHECK-NEXT: i32 3, label [[CASE3]] +; CHECK-NEXT: i32 4, label [[DEFAULT]] ; CHECK-NEXT: ] ; CHECK: default: ; CHECK-NEXT: call void @bar(i32 [[X]]) @@ -521,15 +521,16 @@ define i32 @test13(ptr %ptr1, ptr %ptr2) { ; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i32, ptr [[PTR2:%.*]], i32 1 ; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i32, ptr [[PTR2]], i32 2 ; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[PTR1:%.*]], [[PTR2]] -; CHECK-NEXT: [[VAL2_PRE:%.*]] = load i32, ptr [[GEP2]], align 4 ; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[END:%.*]] ; CHECK: if: +; CHECK-NEXT: [[VAL1:%.*]] = load i32, ptr [[GEP2]], align 4 ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[PHI1:%.*]] = phi ptr [ [[PTR2]], [[IF]] ], [ [[GEP1]], [[ENTRY:%.*]] ] -; CHECK-NEXT: [[PHI2:%.*]] = phi i32 [ [[VAL2_PRE]], [[IF]] ], [ 0, [[ENTRY]] ] +; CHECK-NEXT: [[PHI1:%.*]] = phi ptr [ [[PTR1]], [[IF]] ], [ [[GEP1]], [[ENTRY:%.*]] ] +; CHECK-NEXT: [[PHI2:%.*]] = phi i32 [ [[VAL1]], [[IF]] ], [ 0, [[ENTRY]] ] ; CHECK-NEXT: store i32 0, ptr [[PHI1]], align 4 -; CHECK-NEXT: [[RET:%.*]] = add i32 [[PHI2]], [[VAL2_PRE]] +; CHECK-NEXT: [[VAL2:%.*]] = load i32, ptr [[GEP2]], align 4 +; CHECK-NEXT: [[RET:%.*]] = add i32 [[PHI2]], [[VAL2]] ; CHECK-NEXT: ret i32 [[RET]] ; entry: @@ -552,14 +553,14 @@ end: ret i32 %ret } -define void @test14(ptr %ptr1, ptr noalias %ptr2) { +define void @test14(ptr %ptr1, ptr noalias %ptr2, i1 %b1, i1 %b2) { ; CHECK-LABEL: @test14( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i32, ptr [[PTR1:%.*]], i32 1 ; CHECK-NEXT: [[GEP2:%.*]] = getelementptr inbounds i32, ptr [[PTR1]], i32 2 ; CHECK-NEXT: br label [[LOOP:%.*]] ; CHECK: loop: -; CHECK-NEXT: br i1 undef, label [[LOOP_IF1_CRIT_EDGE:%.*]], label [[THEN:%.*]] +; CHECK-NEXT: br i1 [[B1:%.*]], label [[LOOP_IF1_CRIT_EDGE:%.*]], label [[THEN:%.*]] ; CHECK: loop.if1_crit_edge: ; CHECK-NEXT: [[VAL2_PRE:%.*]] = load i32, ptr [[GEP2]], align 4 ; CHECK-NEXT: br label [[IF1:%.*]] @@ -574,10 +575,10 @@ define void @test14(ptr %ptr1, ptr noalias %ptr2) { ; CHECK: if2: ; CHECK-NEXT: br label [[LOOP_END]] ; CHECK: loop.end: -; CHECK-NEXT: [[PHI3:%.*]] = phi ptr [ [[PTR2]], [[THEN]] ], [ [[PTR1]], [[IF2]] ] +; CHECK-NEXT: [[PHI3:%.*]] = phi ptr [ [[GEP2]], [[THEN]] ], [ [[PTR1]], [[IF2]] ] ; CHECK-NEXT: [[VAL3]] = load i32, ptr [[GEP2]], align 4 ; CHECK-NEXT: store i32 [[VAL3]], ptr [[PHI3]], align 4 -; CHECK-NEXT: br i1 undef, label [[LOOP]], label [[IF1]] +; CHECK-NEXT: br i1 [[B2:%.*]], label [[LOOP]], label [[IF1]] ; entry: %gep1 = getelementptr inbounds i32, ptr %ptr1, i32 1 @@ -586,7 +587,7 @@ entry: loop: %phi1 = phi ptr [ %gep3, %loop.end ], [ %gep1, %entry ] - br i1 undef, label %if1, label %then + br i1 %b1, label %if1, label %then if1: @@ -607,5 +608,201 @@ loop.end: %val3 = load i32, ptr %gep2, align 4 store i32 %val3, ptr %phi3, align 4 %gep3 = getelementptr inbounds i32, ptr %ptr1, i32 1 - br i1 undef, label %loop, label %if1 + br i1 %b2, label %loop, label %if1 +} + +; Make sure that the call to use_ptr does not have %p1 +define void @single_phi1(ptr %p0, ptr %p1, i8 %s) { +; CHECK-LABEL: @single_phi1( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P0:%.*]], align 8 +; CHECK-NEXT: [[CMP1:%.*]] = icmp eq ptr [[P2]], [[P1:%.*]] +; CHECK-NEXT: br i1 [[CMP1]], label [[BB4:%.*]], label [[BB1:%.*]] +; CHECK: bb1: +; CHECK-NEXT: switch i8 [[S:%.*]], label [[BB2:%.*]] [ +; CHECK-NEXT: i8 0, label [[BB1]] +; CHECK-NEXT: i8 1, label [[BB3:%.*]] +; CHECK-NEXT: ] +; CHECK: bb2: +; CHECK-NEXT: unreachable +; CHECK: bb3: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb4: +; CHECK-NEXT: call void @use_bool(i1 [[CMP1]]) +; CHECK-NEXT: call void @use_ptr(ptr [[P2]]) +; CHECK-NEXT: ret void +; +entry: + %p2 = load ptr, ptr %p0, align 8 + %cmp1 = icmp eq ptr %p2, %p1 + br i1 %cmp1, label %bb4, label %bb1 + +bb1: + switch i8 %s, label %bb2 [ + i8 0, label %bb1 + i8 1, label %bb3 + ] + +bb2: + unreachable + +bb3: + br label %bb4 + +bb4: + %phi1 = phi ptr [ %p2, %entry ], [ %p2, %bb3 ] + %cmp2 = icmp eq ptr %phi1, %p1 + call void @use_bool(i1 %cmp2) + call void @use_ptr(ptr %phi1) + ret void +} + +define void @single_phi2(ptr %p0, ptr %p1, i8 %s) { +; CHECK-LABEL: @single_phi2( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P0:%.*]], align 8 +; CHECK-NEXT: [[CMP1:%.*]] = icmp eq ptr [[P2]], [[P1:%.*]] +; CHECK-NEXT: br i1 [[CMP1]], label [[BB4:%.*]], label [[BB1:%.*]] +; CHECK: bb1: +; CHECK-NEXT: switch i8 [[S:%.*]], label [[BB2:%.*]] [ +; CHECK-NEXT: i8 0, label [[BB1]] +; CHECK-NEXT: i8 1, label [[BB3:%.*]] +; CHECK-NEXT: ] +; CHECK: bb2: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb3: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb4: +; CHECK-NEXT: call void @use_bool(i1 [[CMP1]]) +; CHECK-NEXT: call void @use_ptr(ptr [[P2]]) +; CHECK-NEXT: ret void +; +entry: + %p2 = load ptr, ptr %p0, align 8 + %cmp1 = icmp eq ptr %p2, %p1 + br i1 %cmp1, label %bb4, label %bb1 + +bb1: + switch i8 %s, label %bb2 [ + i8 0, label %bb1 + i8 1, label %bb3 + ] + +bb2: + br label %bb4 + +bb3: + br label %bb4 + +bb4: + %phi1 = phi ptr [ %p2, %entry ], [ %p2, %bb2 ], [ %p2, %bb3 ] + %cmp2 = icmp eq ptr %phi1, %p1 + call void @use_bool(i1 %cmp2) + call void @use_ptr(ptr %phi1) + ret void } + +define void @multiple_phi1(ptr %p0, ptr %p1, i8 %s) { +; CHECK-LABEL: @multiple_phi1( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P0:%.*]], align 8 +; CHECK-NEXT: [[CMP1:%.*]] = icmp eq ptr [[P2]], [[P1:%.*]] +; CHECK-NEXT: br i1 [[CMP1]], label [[BB4:%.*]], label [[BB1:%.*]] +; CHECK: bb1: +; CHECK-NEXT: switch i8 [[S:%.*]], label [[BB2:%.*]] [ +; CHECK-NEXT: i8 0, label [[BB1]] +; CHECK-NEXT: i8 1, label [[BB3:%.*]] +; CHECK-NEXT: ] +; CHECK: bb2: +; CHECK-NEXT: unreachable +; CHECK: bb3: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb4: +; CHECK-NEXT: call void @use_bool(i1 [[CMP1]]) +; CHECK-NEXT: br label [[BB5:%.*]] +; CHECK: bb5: +; CHECK-NEXT: call void @use_ptr(ptr [[P2]]) +; CHECK-NEXT: br label [[BB5]] +; +entry: + %p2 = load ptr, ptr %p0, align 8 + %cmp1 = icmp eq ptr %p2, %p1 + br i1 %cmp1, label %bb4, label %bb1 + +bb1: + switch i8 %s, label %bb2 [ + i8 0, label %bb1 + i8 1, label %bb3 + ] + +bb2: + unreachable + +bb3: + br label %bb4 + +bb4: + %phi1 = phi ptr [ %p2, %entry ], [ poison, %bb3 ] + %cmp2 = icmp eq ptr %phi1, %p1 + call void @use_bool(i1 %cmp2) + br label %bb5 + +bb5: + %phi2 = phi ptr [ poison, %bb5 ], [ %phi1, %bb4 ] + call void @use_ptr(ptr %phi2) + br label %bb5 +} + +define void @multiple_phi2(ptr %p0, ptr %p1, i8 %s) { +; CHECK-LABEL: @multiple_phi2( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P0:%.*]], align 8 +; CHECK-NEXT: [[CMP1:%.*]] = icmp eq ptr [[P2]], [[P1:%.*]] +; CHECK-NEXT: br i1 [[CMP1]], label [[BB4:%.*]], label [[BB1:%.*]] +; CHECK: bb1: +; CHECK-NEXT: switch i8 [[S:%.*]], label [[BB2:%.*]] [ +; CHECK-NEXT: i8 0, label [[BB1]] +; CHECK-NEXT: i8 1, label [[BB3:%.*]] +; CHECK-NEXT: ] +; CHECK: bb2: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb3: +; CHECK-NEXT: br label [[BB4]] +; CHECK: bb4: +; CHECK-NEXT: call void @use_bool(i1 [[CMP1]]) +; CHECK-NEXT: br label [[BB5:%.*]] +; CHECK: bb5: +; CHECK-NEXT: call void @use_ptr(ptr [[P2]]) +; CHECK-NEXT: br label [[BB5]] +; +entry: + %p2 = load ptr, ptr %p0, align 8 + %cmp1 = icmp eq ptr %p2, %p1 + br i1 %cmp1, label %bb4, label %bb1 + +bb1: + switch i8 %s, label %bb2 [ + i8 0, label %bb1 + i8 1, label %bb3 + ] + +bb2: + br label %bb4 + +bb3: + br label %bb4 + +bb4: + %phi1 = phi ptr [ %p2, %entry ], [ %p2, %bb2 ], [ poison, %bb3 ] + %cmp2 = icmp eq ptr %phi1, %p1 + call void @use_bool(i1 %cmp2) + br label %bb5 + +bb5: + %phi2 = phi ptr [ poison, %bb5 ], [ %phi1, %bb4 ] + call void @use_ptr(ptr %phi2) + br label %bb5 +} + +declare void @use_bool(i1) +declare void @use_ptr(ptr) diff --git a/llvm/unittests/Analysis/LoadsTest.cpp b/llvm/unittests/Analysis/LoadsTest.cpp index 0111cfeefa41a..5da3feaf762f3 100644 --- a/llvm/unittests/Analysis/LoadsTest.cpp +++ b/llvm/unittests/Analysis/LoadsTest.cpp @@ -68,35 +68,49 @@ TEST(LoadsTest, CanReplacePointersIfEqual) { R"IR( @y = common global [1 x i32] zeroinitializer, align 4 @x = common global [1 x i32] zeroinitializer, align 4 - declare void @use(i32*) -define void @f(i32* %p) { +define void @f(i32* %p1, i32* %p2, i64 %i) { call void @use(i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0)) - call void @use(i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)) + + %p1_idx = getelementptr inbounds i32, i32* %p1, i64 %i + call void @use(i32* %p1_idx) + + %icmp = icmp eq i32* %p1, getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0) + %ptrInt = ptrtoint i32* %p1 to i64 ret void } )IR"); - const auto &DL = M->getDataLayout(); + const DataLayout &DL = M->getDataLayout(); auto *GV = M->getNamedValue("f"); ASSERT_TRUE(GV); auto *F = dyn_cast(GV); ASSERT_TRUE(F); - // NOTE: the implementation of canReplacePointersIfEqual is incomplete. - // Currently the only the cases it returns false for are really sound and - // returning true means unknown. - Value *P = &*F->arg_begin(); + Value *P1 = &*F->arg_begin(); + Value *P2 = F->getArg(1); + Value *NullPtr = Constant::getNullValue(P1->getType()); auto InstIter = F->front().begin(); - Value *ConstDerefPtr = *cast(&*InstIter)->arg_begin(); - // ConstDerefPtr is a constant pointer that is provably de-referenceable. We - // can replace an arbitrary pointer with it. - EXPECT_TRUE(canReplacePointersIfEqual(P, ConstDerefPtr, DL, nullptr)); + CallInst *UserOfY = cast(&*InstIter); + Value *ConstDerefPtr = UserOfY->getArgOperand(0); + // We cannot replace two pointers in arbitrary instructions unless we are + // replacing with null, a constant dereferencable pointer or they have the + // same underlying object. + EXPECT_FALSE(canReplacePointersIfEqual(ConstDerefPtr, P1, DL)); + EXPECT_FALSE(canReplacePointersIfEqual(P1, P2, DL)); + EXPECT_TRUE(canReplacePointersIfEqual(P1, ConstDerefPtr, DL)); + EXPECT_TRUE(canReplacePointersIfEqual(P1, NullPtr, DL)); + + GetElementPtrInst *BasedOnP1 = cast(&*++InstIter); + EXPECT_TRUE(canReplacePointersIfEqual(BasedOnP1, P1, DL)); + EXPECT_FALSE(canReplacePointersIfEqual(BasedOnP1, P2, DL)); - ++InstIter; - Value *ConstUnDerefPtr = *cast(&*InstIter)->arg_begin(); - // ConstUndDerefPtr is a constant pointer that is provably not - // de-referenceable. We cannot replace an arbitrary pointer with it. - EXPECT_FALSE( - canReplacePointersIfEqual(ConstDerefPtr, ConstUnDerefPtr, DL, nullptr)); + // We can replace two arbitrary pointers in icmp and ptrtoint instructions. + auto P1UseIter = P1->use_begin(); + const Use &PtrToIntUse = *P1UseIter; + const Use &IcmpUse = *++P1UseIter; + const Use &GEPUse = *++P1UseIter; + EXPECT_FALSE(canReplacePointersInUseIfEqual(GEPUse, P2, DL)); + EXPECT_TRUE(canReplacePointersInUseIfEqual(PtrToIntUse, P2, DL)); + EXPECT_TRUE(canReplacePointersInUseIfEqual(IcmpUse, P2, DL)); }