[llvm-branch-commits] [llvm] ef2f843 - Revert "[InstCombine] Check inbounds in load/store of gep null transform (PR48577)"

[Nikita Popov via llvm-branch-commits]

Author: Nikita Popov
Date: 2020-12-24T12:36:56+01:00
New Revision: ef2f843347baca1ece69066a3c3a7afa45532079

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

LOG: Revert "[InstCombine] Check inbounds in load/store of gep null transform (PR48577)"

This reverts commit 899faa50f206073cdd8eeaaa130ffa15f850e656.

Upon further consideration, this does not fix the right issue.
Doing this fold for non-inbounds GEPs is legal, because the
resulting pointer is still based-on null, which has no associated
address range, and as such and access to it is UB.

https://bugs.llvm.org/show_bug.cgi?id=48577#c3

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    llvm/test/Transforms/InstCombine/load.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 71f165abe52e..ac617ecd4fd1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -908,8 +908,7 @@ static bool canSimplifyNullStoreOrGEP(StoreInst &SI) {
 
   auto *Ptr = SI.getPointerOperand();
   if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Ptr))
-    if (GEPI->isInBounds())
-      Ptr = GEPI->getOperand(0);
+    Ptr = GEPI->getOperand(0);
   return (isa<ConstantPointerNull>(Ptr) &&
           !NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace()));
 }
@@ -917,7 +916,7 @@ static bool canSimplifyNullStoreOrGEP(StoreInst &SI) {
 static bool canSimplifyNullLoadOrGEP(LoadInst &LI, Value *Op) {
   if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
     const Value *GEPI0 = GEPI->getOperand(0);
-    if (isa<ConstantPointerNull>(GEPI0) && GEPI->isInBounds() &&
+    if (isa<ConstantPointerNull>(GEPI0) &&
         !NullPointerIsDefined(LI.getFunction(), GEPI->getPointerAddressSpace()))
       return true;
   }

diff  --git a/llvm/test/Transforms/InstCombine/load.ll b/llvm/test/Transforms/InstCombine/load.ll
index e4ba908599c9..a6a2155be0b5 100644
--- a/llvm/test/Transforms/InstCombine/load.ll
+++ b/llvm/test/Transforms/InstCombine/load.ll
@@ -69,9 +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:    [[V:%.*]] = getelementptr i32, i32* null, i64 [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = load i32, i32* [[V]], align 4
-; CHECK-NEXT:    ret i32 [[R]]
+; CHECK-NEXT:    store i32 undef, i32* null, align 536870912
+; CHECK-NEXT:    ret i32 undef
 ;
   %V = getelementptr i32, i32* null, i64 %X
   %R = load i32, i32* %V

diff  --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll
index d3842f4bb469..cda08f89501a 100644
--- a/llvm/test/Transforms/InstCombine/store.ll
+++ b/llvm/test/Transforms/InstCombine/store.ll
@@ -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 24, i32* [[PTR]], align 4
+; CHECK-NEXT:    store i32 undef, i32* [[PTR]], align 4
 ; CHECK-NEXT:    ret void
 ;
   %ptr = getelementptr i32, i32 *null, i64 %offset