From ce192ced2b901be67444c481ab5ca0d731e6d982 Mon Sep 17 00:00:00 2001 From: Juneyoung Lee Date: Sun, 20 Jun 2021 15:00:15 +0900 Subject: [PATCH] [InstCombine] Use poison constant to represent the result of unreachable instrs This patch updates InstCombine to use poison constant to represent the resulting value of (either semantically or syntactically) unreachable instrs, or a don't-care value of an unreachable store instruction. This allows more aggressive folding of unused results, as shown in llvm/test/Transforms/InstCombine/getelementptr.ll . Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D104602 --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 10 ++++++---- llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp | 2 +- .../Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp | 10 +++++----- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 14 +++++++------- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 9 +++++---- llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll | 2 +- llvm/test/Transforms/InstCombine/atomic.ll | 6 +++--- .../test/Transforms/InstCombine/builtin-object-size-ptr.ll | 2 +- llvm/test/Transforms/InstCombine/getelementptr.ll | 10 +++------- llvm/test/Transforms/InstCombine/load.ll | 8 ++++---- llvm/test/Transforms/InstCombine/pr44245.ll | 2 +- llvm/test/Transforms/InstCombine/store.ll | 12 ++++++------ 12 files changed, 43 insertions(+), 44 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 4059a72..c198863 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -2275,10 +2275,10 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) { !CalleeF->isDeclaration()) { Instruction *OldCall = &Call; CreateNonTerminatorUnreachable(OldCall); - // If OldCall does not return void then replaceInstUsesWith undef. + // If OldCall does not return void then replaceInstUsesWith poison. // This allows ValueHandlers and custom metadata to adjust itself. if (!OldCall->getType()->isVoidTy()) - replaceInstUsesWith(*OldCall, UndefValue::get(OldCall->getType())); + replaceInstUsesWith(*OldCall, PoisonValue::get(OldCall->getType())); if (isa(OldCall)) return eraseInstFromFunction(*OldCall); @@ -2291,13 +2291,15 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) { } } + // Calling a null function pointer is undefined if a null address isn't + // dereferenceable. if ((isa(Callee) && !NullPointerIsDefined(Call.getFunction())) || isa(Callee)) { - // If Call does not return void then replaceInstUsesWith undef. + // If Call does not return void then replaceInstUsesWith poison. // This allows ValueHandlers and custom metadata to adjust itself. if (!Call.getType()->isVoidTy()) - replaceInstUsesWith(Call, UndefValue::get(Call.getType())); + replaceInstUsesWith(Call, PoisonValue::get(Call.getType())); if (Call.isTerminator()) { // Can't remove an invoke or callbr because we cannot change the CFG. diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp index 5b99f4d5..a7fae64 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -2548,7 +2548,7 @@ Instruction *InstCombinerImpl::optimizeBitCastFromPhi(CastInst &CI, NewV = combineLoadToNewType(*LI, DestTy); // Remove the old load and its use in the old phi, which itself becomes // dead once the whole transform finishes. - replaceInstUsesWith(*LI, UndefValue::get(LI->getType())); + replaceInstUsesWith(*LI, PoisonValue::get(LI->getType())); eraseInstFromFunction(*LI); } else if (auto *BCI = dyn_cast(V)) { NewV = BCI->getOperand(0); diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index a1cf8e4..e61eadf 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -981,10 +981,10 @@ Instruction *InstCombinerImpl::visitLoadInst(LoadInst &LI) { // that this code is not reachable. We do this instead of inserting // an unreachable instruction directly because we cannot modify the // CFG. - StoreInst *SI = new StoreInst(UndefValue::get(LI.getType()), + StoreInst *SI = new StoreInst(PoisonValue::get(LI.getType()), Constant::getNullValue(Op->getType()), &LI); SI->setDebugLoc(LI.getDebugLoc()); - return replaceInstUsesWith(LI, UndefValue::get(LI.getType())); + return replaceInstUsesWith(LI, PoisonValue::get(LI.getType())); } if (Op->hasOneUse()) { @@ -1332,7 +1332,7 @@ static bool removeBitcastsFromLoadStoreOnMinMax(InstCombinerImpl &IC, IC.Builder.SetInsertPoint(USI); combineStoreToNewValue(IC, *USI, NewLI); } - IC.replaceInstUsesWith(*LI, UndefValue::get(LI->getType())); + IC.replaceInstUsesWith(*LI, PoisonValue::get(LI->getType())); IC.eraseInstFromFunction(*LI); return true; } @@ -1439,8 +1439,8 @@ Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) { // store X, null -> turns into 'unreachable' in SimplifyCFG // store X, GEP(null, Y) -> turns into 'unreachable' in SimplifyCFG if (canSimplifyNullStoreOrGEP(SI)) { - if (!isa(Val)) - return replaceOperand(SI, 0, UndefValue::get(Val->getType())); + if (!isa(Val)) + return replaceOperand(SI, 0, PoisonValue::get(Val->getType())); return nullptr; // Do not modify these! } diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 1c99844..6c000b8 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1119,7 +1119,7 @@ Instruction *InstCombinerImpl::SliceUpIllegalIntegerPHI(PHINode &FirstPhi) { // If we have no users, they must be all self uses, just nuke the PHI. if (PHIUsers.empty()) - return replaceInstUsesWith(FirstPhi, UndefValue::get(FirstPhi.getType())); + return replaceInstUsesWith(FirstPhi, PoisonValue::get(FirstPhi.getType())); // If this phi node is transformable, create new PHIs for all the pieces // extracted out of it. First, sort the users by their offset and size. @@ -1218,11 +1218,11 @@ Instruction *InstCombinerImpl::SliceUpIllegalIntegerPHI(PHINode &FirstPhi) { } // Replace all the remaining uses of the PHI nodes (self uses and the lshrs) - // with undefs. - Value *Undef = UndefValue::get(FirstPhi.getType()); + // with poison. + Value *Poison = PoisonValue::get(FirstPhi.getType()); for (unsigned i = 1, e = PHIsToSlice.size(); i != e; ++i) - replaceInstUsesWith(*PHIsToSlice[i], Undef); - return replaceInstUsesWith(FirstPhi, Undef); + replaceInstUsesWith(*PHIsToSlice[i], Poison); + return replaceInstUsesWith(FirstPhi, Poison); } static Value *SimplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, @@ -1346,7 +1346,7 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) { SmallPtrSet PotentiallyDeadPHIs; PotentiallyDeadPHIs.insert(&PN); if (DeadPHICycle(PU, PotentiallyDeadPHIs)) - return replaceInstUsesWith(PN, UndefValue::get(PN.getType())); + return replaceInstUsesWith(PN, PoisonValue::get(PN.getType())); } // If this phi has a single use, and if that use just computes a value for @@ -1358,7 +1358,7 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) { if (PHIUser->hasOneUse() && (isa(PHIUser) || isa(PHIUser)) && PHIUser->user_back() == &PN) { - return replaceInstUsesWith(PN, UndefValue::get(PN.getType())); + return replaceInstUsesWith(PN, PoisonValue::get(PN.getType())); } // When a PHI is used only to be compared with zero, it is safe to replace // an incoming value proved as known nonzero with any non-zero constant. diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 30b96d3..257931d 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -2672,7 +2672,7 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) { } else { // Casts, GEP, or anything else: we're about to delete this instruction, // so it can not have any valid uses. - replaceInstUsesWith(*I, UndefValue::get(I->getType())); + replaceInstUsesWith(*I, PoisonValue::get(I->getType())); } eraseInstFromFunction(*I); } @@ -2877,8 +2877,8 @@ Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) { return nullptr; // A value may still have uses before we process it here (for example, in - // another unreachable block), so convert those to undef. - replaceInstUsesWith(*Prev, UndefValue::get(Prev->getType())); + // another unreachable block), so convert those to poison. + replaceInstUsesWith(*Prev, PoisonValue::get(Prev->getType())); eraseInstFromFunction(*Prev); return &I; } @@ -3071,7 +3071,8 @@ Instruction *InstCombinerImpl::visitExtractValueInst(ExtractValueInst &EV) { if (*EV.idx_begin() == 0) { Instruction::BinaryOps BinOp = WO->getBinaryOp(); Value *LHS = WO->getLHS(), *RHS = WO->getRHS(); - replaceInstUsesWith(*WO, UndefValue::get(WO->getType())); + // Replace the old instruction's uses with poison. + replaceInstUsesWith(*WO, PoisonValue::get(WO->getType())); eraseInstFromFunction(*WO); return BinaryOperator::Create(BinOp, LHS, RHS); } diff --git a/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll b/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll index 3c85b6e..8c1a59a 100644 --- a/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll +++ b/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll @@ -38,7 +38,7 @@ entry: ; ALL: br i1 %tobool, label %entry.cond.end_crit_edge, label %cond.false, !dbg [[LOC_15_6:![0-9]+]] ; ALL: entry.cond.end_crit_edge: ; GVN: %.pre = load %struct.node*, %struct.node** null, align 8, !dbg [[LOC_16_13:![0-9]+]] -; INSTCOMBINE:store %struct.node* undef, %struct.node** null, align 536870912, !dbg [[LOC_16_13:![0-9]+]] +; INSTCOMBINE:store %struct.node* poison, %struct.node** null, align 536870912, !dbg [[LOC_16_13:![0-9]+]] cond.false: %0 = bitcast %struct.desc* %desc to i8***, !dbg !11 diff --git a/llvm/test/Transforms/InstCombine/atomic.ll b/llvm/test/Transforms/InstCombine/atomic.ll index fc134bc..6004729 100644 --- a/llvm/test/Transforms/InstCombine/atomic.ll +++ b/llvm/test/Transforms/InstCombine/atomic.ll @@ -119,8 +119,8 @@ define i32 @test8(i32* %p) { ; ordering imposed. define i32 @test9() { ; CHECK-LABEL: @test9( -; CHECK-NEXT: store i32 undef, i32* null, align 536870912 -; CHECK-NEXT: ret i32 undef +; CHECK-NEXT: store i32 poison, i32* null, align 536870912 +; CHECK-NEXT: ret i32 poison ; %x = load atomic i32, i32* null unordered, align 4 ret i32 %x @@ -177,7 +177,7 @@ define i32 @test11_no_null_opt() #0 { ; ordering imposed. define i32 @test12() { ; CHECK-LABEL: @test12( -; CHECK-NEXT: store atomic i32 undef, i32* null unordered, align 536870912 +; CHECK-NEXT: store atomic i32 poison, i32* null unordered, align 536870912 ; CHECK-NEXT: ret i32 0 ; store atomic i32 0, i32* null unordered, align 4 diff --git a/llvm/test/Transforms/InstCombine/builtin-object-size-ptr.ll b/llvm/test/Transforms/InstCombine/builtin-object-size-ptr.ll index 7747aea..9096784 100644 --- a/llvm/test/Transforms/InstCombine/builtin-object-size-ptr.ll +++ b/llvm/test/Transforms/InstCombine/builtin-object-size-ptr.ll @@ -60,7 +60,7 @@ define void @unknown_use_of_invariant_start({}** %p) { define {}* @minimal_invariant_start_use(i8 %x) { ; CHECK-LABEL: @minimal_invariant_start_use( -; CHECK-NEXT: ret {}* undef +; CHECK-NEXT: ret {}* poison ; %a = alloca i8 %i = call {}* @llvm.invariant.start.p0i8(i64 1, i8* %a) diff --git a/llvm/test/Transforms/InstCombine/getelementptr.ll b/llvm/test/Transforms/InstCombine/getelementptr.ll index 02259f6..4574907 100644 --- a/llvm/test/Transforms/InstCombine/getelementptr.ll +++ b/llvm/test/Transforms/InstCombine/getelementptr.ll @@ -495,7 +495,7 @@ define i1 @test23() { define void @test25() { ; CHECK-LABEL: @test25( ; CHECK-NEXT: entry: -; CHECK-NEXT: store i64 undef, i64* null, align 536870912 +; CHECK-NEXT: store i64 poison, i64* null, align 536870912 ; CHECK-NEXT: tail call void @foo25(i32 0, i64 0) ; CHECK-NEXT: unreachable ; @@ -613,12 +613,8 @@ declare i32 @printf(i8*, ...) define i32 @test29(i8* %start, i32 %X) nounwind { ; CHECK-LABEL: @test29( ; CHECK-NEXT: entry: -; CHECK-NEXT: store i64 undef, i64* null, align 536870912 -; CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr i8, i8* [[START:%.*]], i64 undef -; CHECK-NEXT: [[TMP0:%.*]] = sext i32 [[X:%.*]] to i64 -; CHECK-NEXT: [[ADD_PTR212:%.*]] = getelementptr i8, i8* [[START]], i64 [[TMP0]] -; CHECK-NEXT: [[CMP214:%.*]] = icmp ugt i8* [[ADD_PTR212]], [[ADD_PTR]] -; CHECK-NEXT: br i1 [[CMP214]], label [[IF_THEN216:%.*]], label [[IF_END363:%.*]] +; CHECK-NEXT: store i64 poison, i64* null, align 536870912 +; CHECK-NEXT: br i1 poison, label [[IF_THEN216:%.*]], label [[IF_END363:%.*]] ; CHECK: if.then216: ; CHECK-NEXT: ret i32 1 ; CHECK: if.end363: diff --git a/llvm/test/Transforms/InstCombine/load.ll b/llvm/test/Transforms/InstCombine/load.ll index a6a2155..38143d9 100644 --- a/llvm/test/Transforms/InstCombine/load.ll +++ b/llvm/test/Transforms/InstCombine/load.ll @@ -59,8 +59,8 @@ define i32 @test5(i1 %C) { define i32 @load_gep_null_inbounds(i64 %X) { ; CHECK-LABEL: @load_gep_null_inbounds( -; CHECK-NEXT: store i32 undef, i32* null, align 536870912 -; CHECK-NEXT: ret i32 undef +; CHECK-NEXT: store i32 poison, i32* null, align 536870912 +; CHECK-NEXT: ret i32 poison ; %V = getelementptr inbounds i32, i32* null, i64 %X %R = load i32, i32* %V @@ -69,8 +69,8 @@ define i32 @load_gep_null_inbounds(i64 %X) { define i32 @load_gep_null_not_inbounds(i64 %X) { ; CHECK-LABEL: @load_gep_null_not_inbounds( -; CHECK-NEXT: store i32 undef, i32* null, align 536870912 -; CHECK-NEXT: ret i32 undef +; CHECK-NEXT: store i32 poison, i32* null, align 536870912 +; CHECK-NEXT: ret i32 poison ; %V = getelementptr i32, i32* null, i64 %X %R = load i32, i32* %V diff --git a/llvm/test/Transforms/InstCombine/pr44245.ll b/llvm/test/Transforms/InstCombine/pr44245.ll index f12bc37..28a4b59 100644 --- a/llvm/test/Transforms/InstCombine/pr44245.ll +++ b/llvm/test/Transforms/InstCombine/pr44245.ll @@ -159,7 +159,7 @@ define void @test_2(i1 %c) local_unnamed_addr { ; CHECK: cond.true133: ; CHECK-NEXT: br label [[COND_END144:%.*]] ; CHECK: cond.false138: -; CHECK-NEXT: store %type_2* undef, %type_2** null, align 536870912 +; CHECK-NEXT: store %type_2* poison, %type_2** null, align 536870912 ; CHECK-NEXT: br label [[COND_END144]] ; CHECK: cond.end144: ; CHECK-NEXT: br label [[WHILE_COND]] diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll index cda08f8..35229b6 100644 --- a/llvm/test/Transforms/InstCombine/store.ll +++ b/llvm/test/Transforms/InstCombine/store.ll @@ -4,7 +4,7 @@ define void @test1(i32* %P) { ; CHECK-LABEL: @test1( ; CHECK-NEXT: store i32 123, i32* undef, align 4 -; CHECK-NEXT: store i32 undef, i32* null, align 536870912 +; CHECK-NEXT: store i32 poison, i32* null, align 536870912 ; CHECK-NEXT: ret void ; store i32 undef, i32* %P @@ -26,7 +26,7 @@ define void @test2(i32* %P) { define void @store_at_gep_off_null_inbounds(i64 %offset) { ; CHECK-LABEL: @store_at_gep_off_null_inbounds( ; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, i32* null, i64 [[OFFSET:%.*]] -; CHECK-NEXT: store i32 undef, i32* [[PTR]], align 4 +; CHECK-NEXT: store i32 poison, i32* [[PTR]], align 4 ; CHECK-NEXT: ret void ; %ptr = getelementptr inbounds i32, i32 *null, i64 %offset @@ -37,7 +37,7 @@ define void @store_at_gep_off_null_inbounds(i64 %offset) { define void @store_at_gep_off_null_not_inbounds(i64 %offset) { ; CHECK-LABEL: @store_at_gep_off_null_not_inbounds( ; CHECK-NEXT: [[PTR:%.*]] = getelementptr i32, i32* null, i64 [[OFFSET:%.*]] -; CHECK-NEXT: store i32 undef, i32* [[PTR]], align 4 +; CHECK-NEXT: store i32 poison, i32* [[PTR]], align 4 ; CHECK-NEXT: ret void ; %ptr = getelementptr i32, i32 *null, i64 %offset @@ -141,14 +141,14 @@ define void @test6(i32 %n, float* %a, i32* %gi) nounwind uwtable ssp { ; CHECK-NEXT: br label [[FOR_COND:%.*]] ; CHECK: for.cond: ; CHECK-NEXT: [[STOREMERGE:%.*]] = phi i32 [ 42, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[FOR_BODY:%.*]] ] -; CHECK-NEXT: store i32 [[STOREMERGE]], i32* [[GI:%.*]], align 4, [[TBAA0:!tbaa !.*]] +; CHECK-NEXT: store i32 [[STOREMERGE]], i32* [[GI:%.*]], align 4, !tbaa [[TBAA0:![0-9]+]] ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[STOREMERGE]], [[N:%.*]] ; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[FOR_END:%.*]] ; CHECK: for.body: ; CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[STOREMERGE]] to i64 ; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A:%.*]], i64 [[IDXPROM]] -; CHECK-NEXT: store float 0.000000e+00, float* [[ARRAYIDX]], align 4, [[TBAA4:!tbaa !.*]] -; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* [[GI]], align 4, [[TBAA0]] +; CHECK-NEXT: store float 0.000000e+00, float* [[ARRAYIDX]], align 4, !tbaa [[TBAA4:![0-9]+]] +; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* [[GI]], align 4, !tbaa [[TBAA0]] ; CHECK-NEXT: [[INC]] = add nsw i32 [[TMP0]], 1 ; CHECK-NEXT: br label [[FOR_COND]] ; CHECK: for.end: -- 2.7.4