[llvm-branch-commits] [llvm] 9017791 - Revert "[InstCombine] Fold gep inbounds of null to null"

[Nikita Popov via llvm-branch-commits]

Author: Nikita Popov
Date: 2020-12-24T10:20:31+01:00
New Revision: 90177912a4dbf425a511caa6420e951a73c5fcf7

URL: https://github.com/llvm/llvm-project/commit/90177912a4dbf425a511caa6420e951a73c5fcf7
DIFF: https://github.com/llvm/llvm-project/commit/90177912a4dbf425a511caa6420e951a73c5fcf7.diff

LOG: 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.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/getelementptr.ll
    llvm/test/Transforms/InstCombine/store.ll

Removed: 
    


################################################################################
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<ConstantPointerNull>(SI.getPointerOperand()) &&
-         !NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace());
+  if (NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace()))
+    return false;
+
+  auto *Ptr = SI.getPointerOperand();
+  if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Ptr))
+    if (GEPI->isInBounds())
+      Ptr = GEPI->getOperand(0);
+  return (isa<ConstantPointerNull>(Ptr) &&
+          !NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace()));
 }
 
 static bool canSimplifyNullLoadOrGEP(LoadInst &LI, Value *Op) {
-  return isa<UndefValue>(Op) ||
-         (isa<ConstantPointerNull>(Op) &&
-          !NullPointerIsDefined(LI.getFunction(), LI.getPointerAddressSpace()));
+  if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
+    const Value *GEPI0 = GEPI->getOperand(0);
+    if (isa<ConstantPointerNull>(GEPI0) && GEPI->isInBounds() &&
+        !NullPointerIsDefined(LI.getFunction(), GEPI->getPointerAddressSpace()))
+      return true;
+  }
+  if (isa<UndefValue>(Op) ||
+      (isa<ConstantPointerNull>(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<ConstantPointerNull>(PtrOp) && GEP.isInBounds() &&
-      !NullPointerIsDefined(GEP.getFunction(),
-                            PtrOp->getType()->getPointerAddressSpace()))
-    if (auto *PtrTy = dyn_cast<PointerType>(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_
diff erent_type(i64 %idx1, i64 %idx2) {
 ; CHECK-LABEL: @gep_null_inbounds_
diff erent_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