caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "[InstCombine] propagate 'nsw' on pointer difference of 'inbounds' geps (PR47430)"

Browse Source 
This reverts commit 324a53205a.

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
This commit is contained in:
Sanjay Patel
2020-09-11 10:51:14 -04:00
parent 4c14ee61b7
commit 6aa3fc4a5b
3 changed files with 9 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
View File

@@ -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, "gepdiff", /* NUW */ false,
GEP1->isInBounds() && GEP2->isInBounds());
Result = Builder.CreateSub(Result, Offset, "gepdiff");
}
// If we have p - gep(p, ...) then we have to negate the result.

16
llvm/test/Transforms/InstCombine/sub-gep.ll
View File

@@ -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:%.*]]

2
llvm/test/Transforms/InstCombine/sub.ll
View File

@@ -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