[llvm] [InstCombine] Make indexed compare fold GEP source type independent (PR #71663)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 27 00:46:55 PST 2023
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/71663
>From 3748dfe442ac338009aea1ea554cd5f9fbcb9a85 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 8 Nov 2023 12:03:17 +0100
Subject: [PATCH 1/5] [InstCombine] Make indexed compare fold GEP source type
independent
The indexed compare fold converts comparisons of GEPs with same
(indirect) base into comparisons of offset. Currently, it only
supports GEPs with the same source element type.
This change makes the transform operate on offsets instead, which
removes the type dependence. To keep closer to the scope of the
original implementation, this keeps the limitation that we should
only have at most one variable index per GEP.
---
.../InstCombine/InstCombineCompares.cpp | 95 +++++++------------
.../InstCombine/indexed-gep-compares.ll | 45 +++++----
.../test/Transforms/InstCombine/opaque-ptr.ll | 21 ++--
3 files changed, 72 insertions(+), 89 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index b6e59f707e78268..9a222bda81d85ab 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -18,6 +18,7 @@
#include "llvm/Analysis/CmpInstAnalysis.h"
#include "llvm/Analysis/ConstantFolding.h"
#include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/Utils/Local.h"
#include "llvm/Analysis/VectorUtils.h"
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/DataLayout.h"
@@ -413,11 +414,12 @@ Instruction *InstCombinerImpl::foldCmpLoadFromIndexedGlobal(
/// Returns true if we can rewrite Start as a GEP with pointer Base
/// and some integer offset. The nodes that need to be re-written
/// for this transformation will be added to Explored.
-static bool canRewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
+static bool canRewriteGEPAsOffset(Value *Start, Value *Base,
const DataLayout &DL,
SetVector<Value *> &Explored) {
SmallVector<Value *, 16> WorkList(1, Start);
Explored.insert(Base);
+ uint64_t IndexSize = DL.getIndexTypeSizeInBits(Start->getType());
// The following traversal gives us an order which can be used
// when doing the final transformation. Since in the final
@@ -447,11 +449,11 @@ static bool canRewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
return false;
if (auto *GEP = dyn_cast<GEPOperator>(V)) {
- // We're limiting the GEP to having one index. This will preserve
- // the original pointer type. We could handle more cases in the
- // future.
- if (GEP->getNumIndices() != 1 || !GEP->isInBounds() ||
- GEP->getSourceElementType() != ElemTy)
+ // Only allow GEPs with at most one variable offset.
+ APInt Offset(IndexSize, 0);
+ MapVector<Value *, APInt> VarOffsets;
+ if (!GEP->collectOffset(DL, IndexSize, VarOffsets, Offset) ||
+ VarOffsets.size() > 1)
return false;
if (!Explored.contains(GEP->getOperand(0)))
@@ -528,7 +530,7 @@ static void setInsertionPoint(IRBuilder<> &Builder, Value *V,
/// Returns a re-written value of Start as an indexed GEP using Base as a
/// pointer.
-static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
+static Value *rewriteGEPAsOffset(Value *Start, Value *Base,
const DataLayout &DL,
SetVector<Value *> &Explored,
InstCombiner &IC) {
@@ -539,8 +541,8 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
// 3. Add the edges for the PHI nodes.
// 4. Emit GEPs to get the original pointers.
// 5. Remove the original instructions.
- Type *IndexType = IntegerType::get(
- Base->getContext(), DL.getIndexTypeSizeInBits(Start->getType()));
+ uint64_t IndexSize = DL.getIndexTypeSizeInBits(Start->getType());
+ Type *IndexType = IntegerType::get(Base->getContext(), IndexSize);
DenseMap<Value *, Value *> NewInsts;
NewInsts[Base] = ConstantInt::getNullValue(IndexType);
@@ -559,29 +561,22 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
// Create all the other instructions.
for (Value *Val : Explored) {
-
if (NewInsts.contains(Val))
continue;
if (auto *GEP = dyn_cast<GEPOperator>(Val)) {
- Value *Index = NewInsts[GEP->getOperand(1)] ? NewInsts[GEP->getOperand(1)]
- : GEP->getOperand(1);
- setInsertionPoint(Builder, GEP);
- // Indices might need to be sign extended. GEPs will magically do
- // this, but we need to do it ourselves here.
- if (Index->getType()->getScalarSizeInBits() !=
- NewInsts[GEP->getOperand(0)]->getType()->getScalarSizeInBits()) {
- Index = Builder.CreateSExtOrTrunc(
- Index, NewInsts[GEP->getOperand(0)]->getType(),
- GEP->getOperand(0)->getName() + ".sext");
- }
+ APInt Offset(IndexSize, 0);
+ MapVector<Value *, APInt> VarOffsets;
+ GEP->collectOffset(DL, IndexSize, VarOffsets, Offset);
- auto *Op = NewInsts[GEP->getOperand(0)];
+ setInsertionPoint(Builder, GEP);
+ Value *Op = NewInsts[GEP->getOperand(0)];
+ Value *OffsetV = emitGEPOffset(&Builder, DL, GEP);
if (isa<ConstantInt>(Op) && cast<ConstantInt>(Op)->isZero())
- NewInsts[GEP] = Index;
+ NewInsts[GEP] = OffsetV;
else
NewInsts[GEP] = Builder.CreateNSWAdd(
- Op, Index, GEP->getOperand(0)->getName() + ".add");
+ Op, OffsetV, GEP->getOperand(0)->getName() + ".add");
continue;
}
if (isa<PHINode>(Val))
@@ -609,23 +604,14 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
}
}
- PointerType *PtrTy = PointerType::get(
- Base->getContext(), Start->getType()->getPointerAddressSpace());
for (Value *Val : Explored) {
if (Val == Base)
continue;
- // Depending on the type, for external users we have to emit
- // a GEP or a GEP + ptrtoint.
setInsertionPoint(Builder, Val, false);
-
- // Cast base to the expected type.
- Value *NewVal = Builder.CreateBitOrPointerCast(
- Base, PtrTy, Start->getName() + "to.ptr");
- NewVal = Builder.CreateInBoundsGEP(ElemTy, NewVal, ArrayRef(NewInsts[Val]),
- Val->getName() + ".ptr");
- NewVal = Builder.CreateBitOrPointerCast(
- NewVal, Val->getType(), Val->getName() + ".conv");
+ // Create GEP for external users.
+ Value *NewVal = Builder.CreateInBoundsGEP(
+ Builder.getInt8Ty(), Base, NewInsts[Val], Val->getName() + ".ptr");
IC.replaceInstUsesWith(*cast<Instruction>(Val), NewVal);
// Add old instruction to worklist for DCE. We don't directly remove it
// here because the original compare is one of the users.
@@ -637,28 +623,18 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
/// Looks through GEPs in order to express the input Value as a constant
/// indexed GEP. Returns a pair containing the GEPs Pointer and Index.
-static std::pair<Value *, Value *>
-getAsConstantIndexedAddress(Type *ElemTy, Value *V, const DataLayout &DL) {
- Type *IndexType = IntegerType::get(V->getContext(),
- DL.getIndexTypeSizeInBits(V->getType()));
-
- Constant *Index = ConstantInt::getNullValue(IndexType);
+static std::pair<Value *, APInt>
+getAsConstantIndexedAddress(Value *V, const DataLayout &DL) {
+ APInt Offset = APInt(DL.getIndexTypeSizeInBits(V->getType()), 0);
while (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
// We accept only inbouds GEPs here to exclude the possibility of
// overflow.
- if (!GEP->isInBounds())
+ if (!GEP->isInBounds() || !GEP->accumulateConstantOffset(DL, Offset))
break;
- if (GEP->hasAllConstantIndices() && GEP->getNumIndices() == 1 &&
- GEP->getSourceElementType() == ElemTy &&
- GEP->getOperand(1)->getType() == IndexType) {
- V = GEP->getOperand(0);
- Constant *GEPIndex = static_cast<Constant *>(GEP->getOperand(1));
- Index = ConstantExpr::getAdd(Index, GEPIndex);
- continue;
- }
- break;
+
+ V = GEP->getPointerOperand();
}
- return {V, Index};
+ return {V, Offset};
}
/// Converts (CMP GEPLHS, RHS) if this change would make RHS a constant.
@@ -675,14 +651,14 @@ static Instruction *transformToIndexedCompare(GEPOperator *GEPLHS, Value *RHS,
if (!GEPLHS->hasAllConstantIndices())
return nullptr;
- Type *ElemTy = GEPLHS->getSourceElementType();
- Value *PtrBase, *Index;
- std::tie(PtrBase, Index) = getAsConstantIndexedAddress(ElemTy, GEPLHS, DL);
+ Value *PtrBase;
+ APInt Offset;
+ std::tie(PtrBase, Offset) = getAsConstantIndexedAddress(GEPLHS, DL);
// The set of nodes that will take part in this transformation.
SetVector<Value *> Nodes;
- if (!canRewriteGEPAsOffset(ElemTy, RHS, PtrBase, DL, Nodes))
+ if (!canRewriteGEPAsOffset(RHS, PtrBase, DL, Nodes))
return nullptr;
// We know we can re-write this as
@@ -691,13 +667,14 @@ static Instruction *transformToIndexedCompare(GEPOperator *GEPLHS, Value *RHS,
// can't have overflow on either side. We can therefore re-write
// this as:
// OFFSET1 cmp OFFSET2
- Value *NewRHS = rewriteGEPAsOffset(ElemTy, RHS, PtrBase, DL, Nodes, IC);
+ Value *NewRHS = rewriteGEPAsOffset(RHS, PtrBase, DL, Nodes, IC);
// RewriteGEPAsOffset has replaced RHS and all of its uses with a re-written
// GEP having PtrBase as the pointer base, and has returned in NewRHS the
// offset. Since Index is the offset of LHS to the base pointer, we will now
// compare the offsets instead of comparing the pointers.
- return new ICmpInst(ICmpInst::getSignedPredicate(Cond), Index, NewRHS);
+ return new ICmpInst(ICmpInst::getSignedPredicate(Cond),
+ IC.Builder.getInt(Offset), NewRHS);
}
/// Fold comparisons between a GEP instruction and something else. At this point
diff --git a/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll b/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
index 80b61b19a2c1938..61cf727fb6c8c8b 100644
--- a/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
+++ b/llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
@@ -6,14 +6,15 @@ target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:32-f3
define ptr at test1(ptr %A, i32 %Offset) {
; CHECK-LABEL: @test1(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i32 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i32 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A:%.*]], i32 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
;
entry:
@@ -34,15 +35,16 @@ bb2:
define ptr at test2(i32 %A, i32 %Offset) {
; CHECK-LABEL: @test2(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i32 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[A:%.*]] to ptr
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A_PTR]], i32 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
;
entry:
@@ -157,16 +159,17 @@ bb2:
define ptr at test4(i16 %A, i32 %Offset) {
; CHECK-LABEL: @test4(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i32 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
; CHECK-NEXT: [[TMP0:%.*]] = zext i16 [[A:%.*]] to i32
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[TMP0]] to ptr
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A_PTR]], i32 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
;
entry:
@@ -195,14 +198,15 @@ define ptr at test5(i32 %Offset) personality ptr @__gxx_personality_v0 {
; CHECK-NEXT: [[A:%.*]] = invoke ptr @fun_ptr()
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
; CHECK: cont:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i32 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[CONT]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[CONT]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A]], i32 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A]], i32 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
; CHECK: lpad:
; CHECK-NEXT: [[L:%.*]] = landingpad { ptr, i32 }
@@ -239,15 +243,16 @@ define ptr at test6(i32 %Offset) personality ptr @__gxx_personality_v0 {
; CHECK-NEXT: [[A:%.*]] = invoke i32 @fun_i32()
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
; CHECK: cont:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i32 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[CONT]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i32 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[CONT]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i32 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[A]] to ptr
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A_PTR]], i32 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
; CHECK: lpad:
; CHECK-NEXT: [[L:%.*]] = landingpad { ptr, i32 }
diff --git a/llvm/test/Transforms/InstCombine/opaque-ptr.ll b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
index 900d3f142a6ff79..4448a49ad92bf5d 100644
--- a/llvm/test/Transforms/InstCombine/opaque-ptr.ll
+++ b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
@@ -382,14 +382,15 @@ define <4 x i1> @compare_geps_same_indices_scalar_vector_base_mismatch(ptr %ptr,
define ptr @indexed_compare(ptr %A, i64 %offset) {
; CHECK-LABEL: @indexed_compare(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i64 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i64 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[OFFSET:%.*]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[RHS_ADD]] = add nsw i64 [[RHS_IDX]], 1
-; CHECK-NEXT: [[COND:%.*]] = icmp sgt i64 [[RHS_IDX]], 100
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i64 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i64 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i64 [[RHS_IDX]], 400
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
-; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i64 [[RHS_IDX]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A:%.*]], i64 [[RHS_IDX]]
; CHECK-NEXT: ret ptr [[RHS_PTR]]
;
entry:
@@ -410,16 +411,16 @@ bb2:
define ptr @indexed_compare_different_types(ptr %A, i64 %offset) {
; CHECK-LABEL: @indexed_compare_different_types(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[TMP:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i64 [[OFFSET:%.*]]
+; CHECK-NEXT: [[TMP_IDX:%.*]] = shl nsw i64 [[OFFSET:%.*]], 2
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
-; CHECK-NEXT: [[RHS:%.*]] = phi ptr [ [[RHS_NEXT:%.*]], [[BB]] ], [ [[TMP]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[LHS:%.*]] = getelementptr inbounds i64, ptr [[A]], i64 100
-; CHECK-NEXT: [[RHS_NEXT]] = getelementptr inbounds i32, ptr [[RHS]], i64 1
-; CHECK-NEXT: [[COND:%.*]] = icmp ult ptr [[LHS]], [[RHS]]
+; CHECK-NEXT: [[RHS_IDX:%.*]] = phi i64 [ [[RHS_ADD:%.*]], [[BB]] ], [ [[TMP_IDX]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RHS_ADD]] = add nsw i64 [[RHS_IDX]], 4
+; CHECK-NEXT: [[COND:%.*]] = icmp sgt i64 [[RHS_IDX]], 800
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
; CHECK: bb2:
-; CHECK-NEXT: ret ptr [[RHS]]
+; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i8, ptr [[A:%.*]], i64 [[RHS_IDX]]
+; CHECK-NEXT: ret ptr [[RHS_PTR]]
;
entry:
%tmp = getelementptr inbounds i32, ptr %A, i64 %offset
>From a2b4bb779a41fb3ac7fb49e294f045dd41a3637b Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 14 Nov 2023 15:08:20 +0100
Subject: [PATCH 2/5] Use stripAndAccumulateConstantOffsets()
---
.../InstCombine/InstCombineCompares.cpp | 27 ++++++-------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 9a222bda81d85ab..bcd7dfc37a43d76 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -621,22 +621,6 @@ static Value *rewriteGEPAsOffset(Value *Start, Value *Base,
return NewInsts[Start];
}
-/// Looks through GEPs in order to express the input Value as a constant
-/// indexed GEP. Returns a pair containing the GEPs Pointer and Index.
-static std::pair<Value *, APInt>
-getAsConstantIndexedAddress(Value *V, const DataLayout &DL) {
- APInt Offset = APInt(DL.getIndexTypeSizeInBits(V->getType()), 0);
- while (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
- // We accept only inbouds GEPs here to exclude the possibility of
- // overflow.
- if (!GEP->isInBounds() || !GEP->accumulateConstantOffset(DL, Offset))
- break;
-
- V = GEP->getPointerOperand();
- }
- return {V, Offset};
-}
-
/// Converts (CMP GEPLHS, RHS) if this change would make RHS a constant.
/// We can look through PHIs, GEPs and casts in order to determine a common base
/// between GEPLHS and RHS.
@@ -651,9 +635,14 @@ static Instruction *transformToIndexedCompare(GEPOperator *GEPLHS, Value *RHS,
if (!GEPLHS->hasAllConstantIndices())
return nullptr;
- Value *PtrBase;
- APInt Offset;
- std::tie(PtrBase, Offset) = getAsConstantIndexedAddress(GEPLHS, DL);
+ APInt Offset(DL.getIndexTypeSizeInBits(GEPLHS->getType()), 0);
+ Value *PtrBase =
+ GEPLHS->stripAndAccumulateConstantOffsets(DL, Offset,
+ /*AllowNonInbounds*/ false);
+
+ // Bail if we looked through addrspacecast.
+ if (PtrBase->getType() != GEPLHS->getType())
+ return nullptr;
// The set of nodes that will take part in this transformation.
SetVector<Value *> Nodes;
>From 79c3ce20b2abc2a4cf71c159eba8c9b784de78f2 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv at gmail.com>
Date: Tue, 21 Nov 2023 18:46:54 +0100
Subject: [PATCH 3/5] remove dead code
---
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index bcd7dfc37a43d76..f7cb5aa26e0fe48 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -541,8 +541,8 @@ static Value *rewriteGEPAsOffset(Value *Start, Value *Base,
// 3. Add the edges for the PHI nodes.
// 4. Emit GEPs to get the original pointers.
// 5. Remove the original instructions.
- uint64_t IndexSize = DL.getIndexTypeSizeInBits(Start->getType());
- Type *IndexType = IntegerType::get(Base->getContext(), IndexSize);
+ Type *IndexType = IntegerType::get(
+ Base->getContext(), DL.getIndexTypeSizeInBits(Start->getType()));
DenseMap<Value *, Value *> NewInsts;
NewInsts[Base] = ConstantInt::getNullValue(IndexType);
@@ -565,10 +565,6 @@ static Value *rewriteGEPAsOffset(Value *Start, Value *Base,
continue;
if (auto *GEP = dyn_cast<GEPOperator>(Val)) {
- APInt Offset(IndexSize, 0);
- MapVector<Value *, APInt> VarOffsets;
- GEP->collectOffset(DL, IndexSize, VarOffsets, Offset);
-
setInsertionPoint(Builder, GEP);
Value *Op = NewInsts[GEP->getOperand(0)];
Value *OffsetV = emitGEPOffset(&Builder, DL, GEP);
>From d3f9de6f1cd8a59b59f83dcbf352da69936939a0 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv at gmail.com>
Date: Tue, 21 Nov 2023 18:49:03 +0100
Subject: [PATCH 4/5] Simplify check for at most one variable index
---
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index f7cb5aa26e0fe48..37f18fb622e9a6a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -419,7 +419,6 @@ static bool canRewriteGEPAsOffset(Value *Start, Value *Base,
SetVector<Value *> &Explored) {
SmallVector<Value *, 16> WorkList(1, Start);
Explored.insert(Base);
- uint64_t IndexSize = DL.getIndexTypeSizeInBits(Start->getType());
// The following traversal gives us an order which can be used
// when doing the final transformation. Since in the final
@@ -450,10 +449,8 @@ static bool canRewriteGEPAsOffset(Value *Start, Value *Base,
if (auto *GEP = dyn_cast<GEPOperator>(V)) {
// Only allow GEPs with at most one variable offset.
- APInt Offset(IndexSize, 0);
- MapVector<Value *, APInt> VarOffsets;
- if (!GEP->collectOffset(DL, IndexSize, VarOffsets, Offset) ||
- VarOffsets.size() > 1)
+ auto IsNonConst = [](Value *V) { return !isa<ConstantInt>(V); };
+ if (count_if(GEP->indices(), IsNonConst) > 1)
return false;
if (!Explored.contains(GEP->getOperand(0)))
>From af6cc8fadbdcef614b55d18ac6167bf150eac93c Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 27 Nov 2023 09:45:58 +0100
Subject: [PATCH 5/5] Restore inbounds check
---
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 37f18fb622e9a6a..69b96445c76e9fb 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -448,9 +448,9 @@ static bool canRewriteGEPAsOffset(Value *Start, Value *Base,
return false;
if (auto *GEP = dyn_cast<GEPOperator>(V)) {
- // Only allow GEPs with at most one variable offset.
+ // Only allow inbounds GEPs with at most one variable offset.
auto IsNonConst = [](Value *V) { return !isa<ConstantInt>(V); };
- if (count_if(GEP->indices(), IsNonConst) > 1)
+ if (!GEP->isInBounds() || count_if(GEP->indices(), IsNonConst) > 1)
return false;
if (!Explored.contains(GEP->getOperand(0)))
More information about the llvm-commits
mailing list