[llvm] 6aa3fc4 - Revert "[InstCombine] propagate 'nsw' on pointer difference of 'inbounds' geps (PR47430)"
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 11 07:55:02 PDT 2020
Author: Sanjay Patel
Date: 2020-09-11T10:54:48-04:00
New Revision: 6aa3fc4a5b88bd0175212e06b183c87cf87c306c
URL: https://github.com/llvm/llvm-project/commit/6aa3fc4a5b88bd0175212e06b183c87cf87c306c
DIFF: https://github.com/llvm/llvm-project/commit/6aa3fc4a5b88bd0175212e06b183c87cf87c306c.diff
LOG: Revert "[InstCombine] propagate 'nsw' on pointer difference of 'inbounds' geps (PR47430)"
This reverts commit 324a53205a3af979e3de109fdd52f91781816cba.
On closer examination of at least one of the test diffs,
this does not appear to be correct in all cases. Even the
existing 'nsw' creation may be wrong based on this example:
https://alive2.llvm.org/ce/z/uL4Hw9
https://alive2.llvm.org/ce/z/fJMKQS
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
llvm/test/Transforms/InstCombine/sub-gep.ll
llvm/test/Transforms/InstCombine/sub.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index a5dd8f6d7c9d..5ce32bc592d0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1671,12 +1671,11 @@ Value *InstCombinerImpl::OptimizePointerDifference(Value *LHS, Value *RHS,
I->getOpcode() == Instruction::Mul)
I->setHasNoUnsignedWrap();
- // If we have a 2nd GEP of the same base pointer, subtract the offsets.
- // If both GEPs are inbounds, then the subtract does not have signed overflow.
+ // If we had a constant expression GEP on the other side offsetting the
+ // pointer, subtract it from the offset we have.
if (GEP2) {
Value *Offset = EmitGEPOffset(GEP2);
- Result = Builder.CreateSub(Result, Offset, "gep
diff ", /* NUW */ false,
- GEP1->isInBounds() && GEP2->isInBounds());
+ Result = Builder.CreateSub(Result, Offset, "gep
diff ");
}
// If we have p - gep(p, ...) then we have to negate the result.
diff --git a/llvm/test/Transforms/InstCombine/sub-gep.ll b/llvm/test/Transforms/InstCombine/sub-gep.ll
index ee0c9ffaa0ef..ce9657433bb7 100644
--- a/llvm/test/Transforms/InstCombine/sub-gep.ll
+++ b/llvm/test/Transforms/InstCombine/sub-gep.ll
@@ -245,7 +245,7 @@ define i64 @test24b(i8* %P, i64 %A){
define i64 @test25(i8* %P, i64 %A){
; CHECK-LABEL: @test25(
; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i64 [[A:%.*]], 1
-; CHECK-NEXT: [[GEPDIFF:%.*]] = add nsw i64 [[B_IDX]], -84
+; CHECK-NEXT: [[GEPDIFF:%.*]] = add i64 [[B_IDX]], -84
; CHECK-NEXT: ret i64 [[GEPDIFF]]
;
%B = getelementptr inbounds [42 x i16], [42 x i16]* @Arr, i64 0, i64 %A
@@ -260,7 +260,7 @@ define i16 @test25_as1(i8 addrspace(1)* %P, i64 %A) {
; CHECK-LABEL: @test25_as1(
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[A:%.*]] to i16
; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i16 [[TMP1]], 1
-; CHECK-NEXT: [[GEPDIFF:%.*]] = add nsw i16 [[B_IDX]], -84
+; CHECK-NEXT: [[GEPDIFF:%.*]] = add i16 [[B_IDX]], -84
; CHECK-NEXT: ret i16 [[GEPDIFF]]
;
%B = getelementptr inbounds [42 x i16], [42 x i16] addrspace(1)* @Arr_as1, i64 0, i64 %A
@@ -272,7 +272,7 @@ define i16 @test25_as1(i8 addrspace(1)* %P, i64 %A) {
define i64 @test30(i8* %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test30(
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[I:%.*]], 2
-; CHECK-NEXT: [[GEPDIFF:%.*]] = sub nsw i64 [[GEP1_IDX]], [[J:%.*]]
+; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[GEP1_IDX]], [[J:%.*]]
; CHECK-NEXT: ret i64 [[GEPDIFF]]
;
%bit = bitcast i8* %foo to i32*
@@ -287,7 +287,7 @@ define i64 @test30(i8* %foo, i64 %i, i64 %j) {
define i16 @test30_as1(i8 addrspace(1)* %foo, i16 %i, i16 %j) {
; CHECK-LABEL: @test30_as1(
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[I:%.*]], 2
-; CHECK-NEXT: [[GEPDIFF:%.*]] = sub nsw i16 [[GEP1_IDX]], [[J:%.*]]
+; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i16 [[GEP1_IDX]], [[J:%.*]]
; CHECK-NEXT: ret i16 [[GEPDIFF]]
;
%bit = bitcast i8 addrspace(1)* %foo to i32 addrspace(1)*
@@ -299,11 +299,9 @@ define i16 @test30_as1(i8 addrspace(1)* %foo, i16 %i, i16 %j) {
ret i16 %sub
}
-; Inbounds translates to 'nsw' on sub
-
define i64 @gep_
diff _both_inbounds(i8* %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @gep_
diff _both_inbounds(
-; CHECK-NEXT: [[GEPDIFF:%.*]] = sub nsw i64 [[I:%.*]], [[J:%.*]]
+; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
; CHECK-NEXT: ret i64 [[GEPDIFF]]
;
%gep1 = getelementptr inbounds i8, i8* %foo, i64 %i
@@ -314,8 +312,6 @@ define i64 @gep_
diff _both_inbounds(i8* %foo, i64 %i, i64 %j) {
ret i64 %sub
}
-; Negative test for 'nsw' - both geps must be inbounds
-
define i64 @gep_
diff _first_inbounds(i8* %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @gep_
diff _first_inbounds(
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
@@ -329,8 +325,6 @@ define i64 @gep_
diff _first_inbounds(i8* %foo, i64 %i, i64 %j) {
ret i64 %sub
}
-; Negative test for 'nsw' - both geps must be inbounds
-
define i64 @gep_
diff _second_inbounds(i8* %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @gep_
diff _second_inbounds(
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
diff --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll
index 0940a08bbb44..98d8a9e6b5ca 100644
--- a/llvm/test/Transforms/InstCombine/sub.ll
+++ b/llvm/test/Transforms/InstCombine/sub.ll
@@ -1077,7 +1077,7 @@ define i64 @test58([100 x [100 x i8]]* %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test58(
; CHECK-NEXT: [[GEP1_OFFS:%.*]] = add i64 [[I:%.*]], 4200
; CHECK-NEXT: [[GEP2_OFFS:%.*]] = add i64 [[J:%.*]], 4200
-; CHECK-NEXT: [[GEPDIFF:%.*]] = sub nsw i64 [[GEP1_OFFS]], [[GEP2_OFFS]]
+; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[GEP1_OFFS]], [[GEP2_OFFS]]
; CHECK-NEXT: ret i64 [[GEPDIFF]]
;
%gep1 = getelementptr inbounds [100 x [100 x i8]], [100 x [100 x i8]]* %foo, i64 0, i64 42, i64 %i
More information about the llvm-commits
mailing list