[llvm] e6b086b - Revert "[InstCombine] Fix miscompile on GEP+load to icmp fold (PR45210)"

Nathan Chancellor via llvm-commits llvm-commits at lists.llvm.org
Mon May 31 20:21:58 PDT 2021


Author: Nathan Chancellor
Date: 2021-05-31T20:21:26-07:00
New Revision: e6b086bef2c0597ed14b2aefbb3f6d74b94fd49e

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

LOG: Revert "[InstCombine] Fix miscompile on GEP+load to icmp fold (PR45210)"

This reverts commit 4f2fd3818b0eb26806f366bc37369349aeedcaf9.

The Linux kernel fails to build after this commit. See
https://reviews.llvm.org/D99481 for a reproducer.

Signed-off-by: Nathan Chancellor <nathan at kernel.org>

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
    llvm/test/Transforms/InstCombine/load-cmp.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 5e60edd89d2a0..9e4d88899b19e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -269,30 +269,14 @@ InstCombinerImpl::foldCmpLoadFromIndexedGlobal(GetElementPtrInst *GEP,
   // order the state machines in complexity of the generated code.
   Value *Idx = GEP->getOperand(2);
 
+  // If the index is larger than the pointer size of the target, truncate the
+  // index down like the GEP would do implicitly.  We don't have to do this for
+  // an inbounds GEP because the index can't be out of range.
   if (!GEP->isInBounds()) {
-    // If the index is larger than the pointer size of the target, truncate the
-    // index down like the GEP would do implicitly.  We don't have to do this
-    // for an inbounds GEP because the index can't be out of range.
     Type *IntPtrTy = DL.getIntPtrType(GEP->getType());
     unsigned PtrSize = IntPtrTy->getIntegerBitWidth();
     if (Idx->getType()->getPrimitiveSizeInBits().getFixedSize() > PtrSize)
       Idx = Builder.CreateTrunc(Idx, IntPtrTy);
-
-    unsigned ElementSize =
-        DL.getTypeAllocSize(Init->getType()->getArrayElementType());
-
-    // If inbounds keyword is not present, Idx * ElementSize can overflow.
-    // Let's assume that ElementSize is 2 and the wanted value is at offset 0.
-    // Then, there are two possible values for Idx to match offset 0:
-    // 0x00..00, 0x80..00.
-    // Emitting 'icmp eq Idx, 0' isn't correct in this case because the
-    // comparison is false if Idx was 0x80..00.
-    // We need to erase the highest countTrailingZeros(ElementSize) bits of Idx.
-    if (countTrailingZeros(ElementSize) != 0) {
-      Value *Mask = ConstantInt::getSigned(Idx->getType(), -1);
-      Mask = Builder.CreateLShr(Mask, countTrailingZeros(ElementSize));
-      Idx = Builder.CreateAnd(Idx, Mask);
-    }
   }
 
   // If the comparison is only true for one or two elements, emit direct

diff  --git a/llvm/test/Transforms/InstCombine/load-cmp.ll b/llvm/test/Transforms/InstCombine/load-cmp.ll
index b56cfdd6c5568..84f692dd4595d 100644
--- a/llvm/test/Transforms/InstCombine/load-cmp.ll
+++ b/llvm/test/Transforms/InstCombine/load-cmp.ll
@@ -33,8 +33,7 @@ define i1 @test1(i32 %X) {
 
 define i1 @test1_noinbounds(i32 %X) {
 ; CHECK-LABEL: @test1_noinbounds(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[X:%.*]], 2147483647
-; CHECK-NEXT:    [[R:%.*]] = icmp eq i32 [[TMP1]], 9
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i32 [[X:%.*]], 9
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %P = getelementptr [10 x i16], [10 x i16]* @G16, i32 0, i32 %X
@@ -45,8 +44,8 @@ define i1 @test1_noinbounds(i32 %X) {
 
 define i1 @test1_noinbounds_i64(i64 %X) {
 ; CHECK-LABEL: @test1_noinbounds_i64(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[X:%.*]], 2147483647
-; CHECK-NEXT:    [[R:%.*]] = icmp eq i64 [[TMP1]], 9
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[X:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i32 [[TMP1]], 9
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %P = getelementptr [10 x i16], [10 x i16]* @G16, i64 0, i64 %X
@@ -55,14 +54,10 @@ define i1 @test1_noinbounds_i64(i64 %X) {
   ret i1 %R
 }
 
-; When x is 0x8009, x * 2(ElementSize) overflows and it becomes 0x0012.
-; It is the same location as when x is 0x0009, So we need to return
-; true if x AND 0x7FFF is 0x0009.
-
 define i1 @test1_noinbounds_as1(i32 %x) {
 ; CHECK-LABEL: @test1_noinbounds_as1(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[X:%.*]], 32767
-; CHECK-NEXT:    [[R:%.*]] = icmp eq i32 [[TMP1]], 9
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i32 [[X:%.*]] to i16
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i16 [[TMP1]], 9
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %p = getelementptr [10 x i16], [10 x i16] addrspace(1)* @G16_as1, i16 0, i32 %x
@@ -265,8 +260,7 @@ define i1 @test10_struct_arr(i32 %x) {
 
 define i1 @test10_struct_arr_noinbounds(i32 %x) {
 ; CHECK-LABEL: @test10_struct_arr_noinbounds(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[X:%.*]], 268435455
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i32 [[TMP1]], 1
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i32 [[X:%.*]], 1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %p = getelementptr [4 x %Foo], [4 x %Foo]* @GStructArr, i32 0, i32 %x, i32 2
@@ -300,9 +294,7 @@ define i1 @test10_struct_arr_i64(i64 %x) {
 
 define i1 @test10_struct_arr_noinbounds_i16(i16 %x) {
 ; CHECK-LABEL: @test10_struct_arr_noinbounds_i16(
-; CHECK-NEXT:    [[TMP1:%.*]] = sext i16 [[X:%.*]] to i32
-; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[TMP1]], 268435455
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i32 [[TMP2]], 1
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i16 [[X:%.*]], 1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %p = getelementptr [4 x %Foo], [4 x %Foo]* @GStructArr, i32 0, i16 %x, i32 2
@@ -313,8 +305,8 @@ define i1 @test10_struct_arr_noinbounds_i16(i16 %x) {
 
 define i1 @test10_struct_arr_noinbounds_i64(i64 %x) {
 ; CHECK-LABEL: @test10_struct_arr_noinbounds_i64(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[X:%.*]], 268435455
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i64 [[TMP1]], 1
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[X:%.*]] to i32
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i32 [[TMP1]], 1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %p = getelementptr [4 x %Foo], [4 x %Foo]* @GStructArr, i32 0, i64 %x, i32 2


        


More information about the llvm-commits mailing list