Skip to content

Commit 8b30067

Browse files
rotaterightrotateright
committed
[InstCombine] improve fold of pointer differences
This was supposed to be an NFC cleanup, but there's a real logic difference (did not drop 'nsw') visible in some tests in addition to an efficiency improvement. This is because in the case where we have 2 GEPs, the code was *always* swapping the operands and negating the result. But if we have 2 GEPs, we should *never* need swapping/negation AFAICT. This is part of improving flags propagation noticed with PR47430.
1 parent 7020781 commit 8b30067

File tree

3 files changed

+29
-45
lines changed

3 files changed

+29
-45
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,43 +1615,27 @@ Value *InstCombinerImpl::OptimizePointerDifference(Value *LHS, Value *RHS,
16151615
// this.
16161616
bool Swapped = false;
16171617
GEPOperator *GEP1 = nullptr, *GEP2 = nullptr;
1618+
if (!isa<GEPOperator>(LHS) && isa<GEPOperator>(RHS)) {
1619+
std::swap(LHS, RHS);
1620+
Swapped = true;
1621+
}
16181622

1619-
// For now we require one side to be the base pointer "A" or a constant
1620-
// GEP derived from it.
1621-
if (GEPOperator *LHSGEP = dyn_cast<GEPOperator>(LHS)) {
1623+
// Require at least one GEP with a common base pointer on both sides.
1624+
if (auto *LHSGEP = dyn_cast<GEPOperator>(LHS)) {
16221625
// (gep X, ...) - X
16231626
if (LHSGEP->getOperand(0) == RHS) {
16241627
GEP1 = LHSGEP;
1625-
Swapped = false;
1626-
} else if (GEPOperator *RHSGEP = dyn_cast<GEPOperator>(RHS)) {
1628+
} else if (auto *RHSGEP = dyn_cast<GEPOperator>(RHS)) {
16271629
// (gep X, ...) - (gep X, ...)
16281630
if (LHSGEP->getOperand(0)->stripPointerCasts() ==
1629-
RHSGEP->getOperand(0)->stripPointerCasts()) {
1630-
GEP2 = RHSGEP;
1631+
RHSGEP->getOperand(0)->stripPointerCasts()) {
16311632
GEP1 = LHSGEP;
1632-
Swapped = false;
1633-
}
1634-
}
1635-
}
1636-
1637-
if (GEPOperator *RHSGEP = dyn_cast<GEPOperator>(RHS)) {
1638-
// X - (gep X, ...)
1639-
if (RHSGEP->getOperand(0) == LHS) {
1640-
GEP1 = RHSGEP;
1641-
Swapped = true;
1642-
} else if (GEPOperator *LHSGEP = dyn_cast<GEPOperator>(LHS)) {
1643-
// (gep X, ...) - (gep X, ...)
1644-
if (RHSGEP->getOperand(0)->stripPointerCasts() ==
1645-
LHSGEP->getOperand(0)->stripPointerCasts()) {
1646-
GEP2 = LHSGEP;
1647-
GEP1 = RHSGEP;
1648-
Swapped = true;
1633+
GEP2 = RHSGEP;
16491634
}
16501635
}
16511636
}
16521637

16531638
if (!GEP1)
1654-
// No GEP found.
16551639
return nullptr;
16561640

16571641
if (GEP2) {

‎llvm/test/Transforms/InstCombine/sub-gep.ll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,10 @@ define i64 @test_inbounds2_nuw_swapped([0 x i32]* %base, i64 %idx) {
124124
; The sub and shl here could be nuw, but this is harder to handle.
125125
define i64 @test_inbounds_nuw_two_gep([0 x i32]* %base, i64 %idx, i64 %idx2) {
126126
; CHECK-LABEL: @test_inbounds_nuw_two_gep(
127+
; CHECK-NEXT: [[P2_IDX:%.*]] = shl nsw i64 [[IDX2:%.*]], 2
127128
; CHECK-NEXT: [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
128-
; CHECK-NEXT: [[P2_IDX_NEG_NEG:%.*]] = shl i64 [[IDX2:%.*]], 2
129-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = add i64 [[P2_IDX_NEG_NEG]], [[P1_IDX_NEG]]
130-
; CHECK-NEXT: ret i64 [[GEPDIFF_NEG]]
129+
; CHECK-NEXT: [[GEPDIFF:%.*]] = add i64 [[P1_IDX_NEG]], [[P2_IDX]]
130+
; CHECK-NEXT: ret i64 [[GEPDIFF]]
131131
;
132132
%p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64 %idx
133133
%p2 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64 %idx2

‎llvm/test/Transforms/InstCombine/sub.ll‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,9 @@ define i64 @test24b(i8* %P, i64 %A){
505505

506506
define i64 @test25(i8* %P, i64 %A){
507507
; CHECK-LABEL: @test25(
508-
; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i64 [[A:%.*]], 1
509-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = add i64 [[B_IDX_NEG_NEG]], -84
510-
; CHECK-NEXT: ret i64 [[GEPDIFF_NEG]]
508+
; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i64 [[A:%.*]], 1
509+
; CHECK-NEXT: [[GEPDIFF:%.*]] = add i64 [[B_IDX]], -84
510+
; CHECK-NEXT: ret i64 [[GEPDIFF]]
511511
;
512512
%B = getelementptr inbounds [42 x i16], [42 x i16]* @Arr, i64 0, i64 %A
513513
%C = ptrtoint i16* %B to i64
@@ -520,9 +520,9 @@ define i64 @test25(i8* %P, i64 %A){
520520
define i16 @test25_as1(i8 addrspace(1)* %P, i64 %A) {
521521
; CHECK-LABEL: @test25_as1(
522522
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[A:%.*]] to i16
523-
; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i16 [[TMP1]], 1
524-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = add i16 [[B_IDX_NEG_NEG]], -84
525-
; CHECK-NEXT: ret i16 [[GEPDIFF_NEG]]
523+
; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i16 [[TMP1]], 1
524+
; CHECK-NEXT: [[GEPDIFF:%.*]] = add i16 [[B_IDX]], -84
525+
; CHECK-NEXT: ret i16 [[GEPDIFF]]
526526
;
527527
%B = getelementptr inbounds [42 x i16], [42 x i16] addrspace(1)* @Arr_as1, i64 0, i64 %A
528528
%C = ptrtoint i16 addrspace(1)* %B to i16
@@ -825,8 +825,8 @@ define i32 @test28commuted(i32 %x, i32 %y, i32 %z) {
825825

826826
define i64 @test29(i8* %foo, i64 %i, i64 %j) {
827827
; CHECK-LABEL: @test29(
828-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
829-
; CHECK-NEXT: ret i64 [[GEPDIFF_NEG]]
828+
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
829+
; CHECK-NEXT: ret i64 [[GEPDIFF]]
830830
;
831831
%gep1 = getelementptr inbounds i8, i8* %foo, i64 %i
832832
%gep2 = getelementptr inbounds i8, i8* %foo, i64 %j
@@ -838,9 +838,9 @@ define i64 @test29(i8* %foo, i64 %i, i64 %j) {
838838

839839
define i64 @test30(i8* %foo, i64 %i, i64 %j) {
840840
; CHECK-LABEL: @test30(
841-
; CHECK-NEXT: [[GEP1_IDX_NEG_NEG:%.*]] = shl i64 [[I:%.*]], 2
842-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = sub i64 [[GEP1_IDX_NEG_NEG]], [[J:%.*]]
843-
; CHECK-NEXT: ret i64 [[GEPDIFF_NEG]]
841+
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[I:%.*]], 2
842+
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[GEP1_IDX]], [[J:%.*]]
843+
; CHECK-NEXT: ret i64 [[GEPDIFF]]
844844
;
845845
%bit = bitcast i8* %foo to i32*
846846
%gep1 = getelementptr inbounds i32, i32* %bit, i64 %i
@@ -853,9 +853,9 @@ define i64 @test30(i8* %foo, i64 %i, i64 %j) {
853853

854854
define i16 @test30_as1(i8 addrspace(1)* %foo, i16 %i, i16 %j) {
855855
; CHECK-LABEL: @test30_as1(
856-
; CHECK-NEXT: [[GEP1_IDX_NEG_NEG:%.*]] = shl i16 [[I:%.*]], 2
857-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = sub i16 [[GEP1_IDX_NEG_NEG]], [[J:%.*]]
858-
; CHECK-NEXT: ret i16 [[GEPDIFF_NEG]]
856+
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[I:%.*]], 2
857+
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i16 [[GEP1_IDX]], [[J:%.*]]
858+
; CHECK-NEXT: ret i16 [[GEPDIFF]]
859859
;
860860
%bit = bitcast i8 addrspace(1)* %foo to i32 addrspace(1)*
861861
%gep1 = getelementptr inbounds i32, i32 addrspace(1)* %bit, i16 %i
@@ -1234,10 +1234,10 @@ define i64 @test58([100 x [100 x i8]]* %foo, i64 %i, i64 %j) {
12341234
; "%sub = i64 %i, %j, ret i64 %sub"
12351235
; gep1 and gep2 have only one use
12361236
; CHECK-LABEL: @test58(
1237-
; CHECK-NEXT: [[GEP2_OFFS:%.*]] = add i64 [[J:%.*]], 4200
12381237
; CHECK-NEXT: [[GEP1_OFFS:%.*]] = add i64 [[I:%.*]], 4200
1239-
; CHECK-NEXT: [[GEPDIFF_NEG:%.*]] = sub i64 [[GEP1_OFFS]], [[GEP2_OFFS]]
1240-
; CHECK-NEXT: ret i64 [[GEPDIFF_NEG]]
1238+
; CHECK-NEXT: [[GEP2_OFFS:%.*]] = add i64 [[J:%.*]], 4200
1239+
; CHECK-NEXT: [[GEPDIFF:%.*]] = sub i64 [[GEP1_OFFS]], [[GEP2_OFFS]]
1240+
; CHECK-NEXT: ret i64 [[GEPDIFF]]
12411241
;
12421242
%gep1 = getelementptr inbounds [100 x [100 x i8]], [100 x [100 x i8]]* %foo, i64 0, i64 42, i64 %i
12431243
%gep2 = getelementptr inbounds [100 x [100 x i8]], [100 x [100 x i8]]* %foo, i64 0, i64 42, i64 %j

0 commit comments

Comments
 (0)