[llvm] [InstCombine] Remove one-use restriction on icmp of gep fold (PR #76730)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 3 00:17:29 PST 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/76730
>From bd9ca79c0aa0b74977bf99d8ce4350e97160c8d9 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Fri, 8 Dec 2023 16:03:01 +0100
Subject: [PATCH 1/2] [InstCombine] Remove one-use restriction on icmp of gep
fold
The fold for icmp (gep (p, i1), gep (p, i2)) to icmp (i1, i2)
is currently limited to one of the GEPs either having one use or
a constant offset. I believe this is to avoid duplicating complex
arithmetic both in the GEP and the offset comparison.
This patch instead does the same thing that the indexed compare
fold does, which is to rewrite the GEP into i8 form if necessary,
so that the offset arithmetic is not repeated after the transform.
---
.../InstCombine/InstCombineCompares.cpp | 28 ++++++++++++++-----
.../Transforms/InstCombine/icmp-custom-dl.ll | 4 +--
llvm/test/Transforms/InstCombine/icmp-gep.ll | 17 ++++++-----
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 3875e59c3ede3b..ce928e40cdade0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -813,14 +813,28 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
}
}
- // Only lower this if the icmp is the only user of the GEP or if we expect
- // the result to fold to a constant!
- if ((GEPsInBounds || CmpInst::isEquality(Cond)) &&
- (GEPLHS->hasAllConstantIndices() || GEPLHS->hasOneUse()) &&
- (GEPRHS->hasAllConstantIndices() || GEPRHS->hasOneUse())) {
+ if (GEPsInBounds || CmpInst::isEquality(Cond)) {
+ auto EmitGEPOffsetAndRewrite = [&](GEPOperator *GEP) {
+ auto *Inst = dyn_cast<Instruction>(GEP);
+ if (Inst)
+ Builder.SetInsertPoint(Inst);
+
+ Value *Offset = EmitGEPOffset(GEP);
+ // If a non-trivial GEP has other uses, rewrite it to avoid duplicating
+ // the offset arithmetic.
+ if (Inst && !GEP->hasOneUse() && !GEP->hasAllConstantIndices()) {
+ replaceInstUsesWith(*Inst,
+ Builder.CreateGEP(Builder.getInt8Ty(),
+ GEP->getPointerOperand(),
+ Offset, "", GEPsInBounds));
+ eraseInstFromFunction(*Inst);
+ }
+ return Offset;
+ };
+
// ((gep Ptr, OFFSET1) cmp (gep Ptr, OFFSET2) ---> (OFFSET1 cmp OFFSET2)
- Value *L = EmitGEPOffset(GEPLHS);
- Value *R = EmitGEPOffset(GEPRHS);
+ Value *L = EmitGEPOffsetAndRewrite(GEPLHS);
+ Value *R = EmitGEPOffsetAndRewrite(GEPRHS);
return new ICmpInst(ICmpInst::getSignedPredicate(Cond), L, R);
}
}
diff --git a/llvm/test/Transforms/InstCombine/icmp-custom-dl.ll b/llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
index 330825e1055eaa..f0e8962b1b6a64 100644
--- a/llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
@@ -40,8 +40,8 @@ define i1 @test59_as1(ptr addrspace(1) %foo) {
define i1 @test60(ptr %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test60(
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[I:%.*]] to i32
-; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i32
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i32 [[TMP1]], 2
+; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i32
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[GEP1_IDX]], [[TMP2]]
; CHECK-NEXT: ret i1 [[CMP]]
;
@@ -54,8 +54,8 @@ define i1 @test60(ptr %foo, i64 %i, i64 %j) {
define i1 @test60_as1(ptr addrspace(1) %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test60_as1(
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[I:%.*]] to i16
-; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i16
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[TMP1]], 2
+; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i16
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i16 [[GEP1_IDX]], [[TMP2]]
; CHECK-NEXT: ret i1 [[CMP]]
;
diff --git a/llvm/test/Transforms/InstCombine/icmp-gep.ll b/llvm/test/Transforms/InstCombine/icmp-gep.ll
index fb297e04ceb065..42e15e78975e51 100644
--- a/llvm/test/Transforms/InstCombine/icmp-gep.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-gep.ll
@@ -313,8 +313,8 @@ define i1 @test_gep_eq_no_inbounds(ptr %foo, i64 %i, i64 %j) {
define i1 @test60_as1(ptr addrspace(1) %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test60_as1(
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[I:%.*]] to i16
-; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i16
; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[TMP1]], 2
+; CHECK-NEXT: [[TMP2:%.*]] = trunc i64 [[J:%.*]] to i16
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i16 [[GEP1_IDX]], [[TMP2]]
; CHECK-NEXT: ret i1 [[CMP]]
;
@@ -400,11 +400,13 @@ define i1 @test61_as1(ptr addrspace(1) %foo, i16 %i, i16 %j) {
define i1 @test60_extra_use(ptr %foo, i64 %i, i64 %j) {
; CHECK-LABEL: @test60_extra_use(
-; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i32, ptr [[FOO:%.*]], i64 [[I:%.*]]
-; CHECK-NEXT: [[GEP2:%.*]] = getelementptr inbounds i16, ptr [[FOO]], i64 [[J:%.*]]
+; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[I:%.*]], 2
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i8, ptr [[FOO:%.*]], i64 [[GEP1_IDX]]
+; CHECK-NEXT: [[GEP2_IDX:%.*]] = shl nsw i64 [[J:%.*]], 1
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr inbounds i8, ptr [[FOO]], i64 [[GEP2_IDX]]
; CHECK-NEXT: call void @use(ptr [[GEP1]])
; CHECK-NEXT: call void @use(ptr [[GEP2]])
-; CHECK-NEXT: [[CMP:%.*]] = icmp ult ptr [[GEP1]], [[GEP2]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i64 [[GEP1_IDX]], [[GEP2_IDX]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%gep1 = getelementptr inbounds i32, ptr %foo, i64 %i
@@ -446,13 +448,14 @@ define i1 @test60_extra_use_const_operands_no_inbounds(ptr %foo, i64 %i, i64 %j)
define void @test60_extra_use_fold(ptr %foo, i64 %start.idx, i64 %end.offset) {
; CHECK-LABEL: @test60_extra_use_fold(
-; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i32, ptr [[FOO:%.*]], i64 [[START_IDX:%.*]]
+; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[START_IDX:%.*]], 2
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i8, ptr [[FOO:%.*]], i64 [[GEP1_IDX]]
; CHECK-NEXT: [[GEP2:%.*]] = getelementptr inbounds i8, ptr [[FOO]], i64 [[END_OFFSET:%.*]]
; CHECK-NEXT: call void @use(ptr [[GEP1]])
; CHECK-NEXT: call void @use(ptr [[GEP2]])
-; CHECK-NEXT: [[CMP1:%.*]] = icmp eq ptr [[GEP1]], [[GEP2]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i64 [[GEP1_IDX]], [[END_OFFSET]]
; CHECK-NEXT: call void @use.i1(i1 [[CMP1]])
-; CHECK-NEXT: [[CMP2:%.*]] = icmp ult ptr [[GEP1]], [[GEP2]]
+; CHECK-NEXT: [[CMP2:%.*]] = icmp slt i64 [[GEP1_IDX]], [[END_OFFSET]]
; CHECK-NEXT: call void @use.i1(i1 [[CMP2]])
; CHECK-NEXT: ret void
;
>From efe5e2dce0320025162e3ac12b57598c8bfe37ca Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 3 Jan 2024 09:17:05 +0100
Subject: [PATCH 2/2] Don't recreate GEP if it already has i8 type
---
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index ce928e40cdade0..33141b7637e738 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -822,7 +822,8 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
Value *Offset = EmitGEPOffset(GEP);
// If a non-trivial GEP has other uses, rewrite it to avoid duplicating
// the offset arithmetic.
- if (Inst && !GEP->hasOneUse() && !GEP->hasAllConstantIndices()) {
+ if (Inst && !GEP->hasOneUse() && !GEP->hasAllConstantIndices() &&
+ !GEP->getSourceElementType()->isIntegerTy(8)) {
replaceInstUsesWith(*Inst,
Builder.CreateGEP(Builder.getInt8Ty(),
GEP->getPointerOperand(),
More information about the llvm-commits
mailing list