From 90177912a4dbf425a511caa6420e951a73c5fcf7 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 24 Dec 2020 10:19:11 +0100 Subject: [PATCH] Revert "[InstCombine] Fold gep inbounds of null to null" This reverts commit eb79fd3c928dbbb97f7937963361c1dad2bf8222. This causes stage2 crashes, possibly due to StringMap being miscompiled. Reverting for now. --- .../InstCombineLoadStoreAlloca.cpp | 25 +++++++++++++++---- .../InstCombine/InstructionCombining.cpp | 7 ------ .../Transforms/InstCombine/getelementptr.ll | 6 +++-- llvm/test/Transforms/InstCombine/store.ll | 3 ++- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index 153947802f80..71f165abe52e 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -903,14 +903,29 @@ static Instruction *replaceGEPIdxWithZero(InstCombinerImpl &IC, Value *Ptr, } static bool canSimplifyNullStoreOrGEP(StoreInst &SI) { - return isa(SI.getPointerOperand()) && - !NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace()); + if (NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace())) + return false; + + auto *Ptr = SI.getPointerOperand(); + if (GetElementPtrInst *GEPI = dyn_cast(Ptr)) + if (GEPI->isInBounds()) + Ptr = GEPI->getOperand(0); + return (isa(Ptr) && + !NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace())); } static bool canSimplifyNullLoadOrGEP(LoadInst &LI, Value *Op) { - return isa(Op) || - (isa(Op) && - !NullPointerIsDefined(LI.getFunction(), LI.getPointerAddressSpace())); + if (GetElementPtrInst *GEPI = dyn_cast(Op)) { + const Value *GEPI0 = GEPI->getOperand(0); + if (isa(GEPI0) && GEPI->isInBounds() && + !NullPointerIsDefined(LI.getFunction(), GEPI->getPointerAddressSpace())) + return true; + } + if (isa(Op) || + (isa(Op) && + !NullPointerIsDefined(LI.getFunction(), LI.getPointerAddressSpace()))) + return true; + return false; } Instruction *InstCombinerImpl::visitLoadInst(LoadInst &LI) { diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8ace625e1fad..ec932aaf0b9e 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1855,13 +1855,6 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) { Value *PtrOp = GEP.getOperand(0); - // The only pointer that is inbounds of null is null. - if (isa(PtrOp) && GEP.isInBounds() && - !NullPointerIsDefined(GEP.getFunction(), - PtrOp->getType()->getPointerAddressSpace())) - if (auto *PtrTy = dyn_cast(GEPType)) - return replaceInstUsesWith(GEP, ConstantPointerNull::get(PtrTy)); - // Eliminate unneeded casts for indices, and replace indices which displace // by multiples of a zero size type with zero. bool MadeChange = false; diff --git a/llvm/test/Transforms/InstCombine/getelementptr.ll b/llvm/test/Transforms/InstCombine/getelementptr.ll index f1af3fda79e7..1ceba393aff6 100644 --- a/llvm/test/Transforms/InstCombine/getelementptr.ll +++ b/llvm/test/Transforms/InstCombine/getelementptr.ll @@ -1239,7 +1239,8 @@ define i32* @PR45084_extra_use(i1 %cond, %struct.f** %p) { define i8* @gep_null_inbounds(i64 %idx) { ; CHECK-LABEL: @gep_null_inbounds( -; CHECK-NEXT: ret i8* null +; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i8, i8* null, i64 [[IDX:%.*]] +; CHECK-NEXT: ret i8* [[GEP]] ; %gep = getelementptr inbounds i8, i8* null, i64 %idx ret i8* %gep @@ -1265,7 +1266,8 @@ define i8* @gep_null_defined(i64 %idx) null_pointer_is_valid { define i8* @gep_null_inbounds_different_type(i64 %idx1, i64 %idx2) { ; CHECK-LABEL: @gep_null_inbounds_different_type( -; CHECK-NEXT: ret i8* null +; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds [0 x i8], [0 x i8]* null, i64 0, i64 [[IDX2:%.*]] +; CHECK-NEXT: ret i8* [[GEP]] ; %gep = getelementptr inbounds [0 x i8], [0 x i8]* null, i64 %idx1, i64 %idx2 ret i8* %gep diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll index a94ce92d214e..d3842f4bb469 100644 --- a/llvm/test/Transforms/InstCombine/store.ll +++ b/llvm/test/Transforms/InstCombine/store.ll @@ -25,7 +25,8 @@ 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: store i32 undef, i32* null, align 536870912 +; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, i32* null, i64 [[OFFSET:%.*]] +; CHECK-NEXT: store i32 undef, i32* [[PTR]], align 4 ; CHECK-NEXT: ret void ; %ptr = getelementptr inbounds i32, i32 *null, i64 %offset