Skip to content

[InstCombine] Make indexed compare fold GEP source type independent #71663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 28, 2023
Merged

[InstCombine] Make indexed compare fold GEP source type independent #71663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3748dfe
[InstCombine] Make indexed compare fold GEP source type independent
nikic Nov 8, 2023
a2b4bb7
Use stripAndAccumulateConstantOffsets()
nikic Nov 14, 2023
79c3ce2
remove dead code
nikic Nov 21, 2023
d3f9de6
Simplify check for at most one variable index
nikic Nov 21, 2023
af6cc8f
Restore inbounds check
nikic Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 25 additions & 66 deletions llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -413,7 +414,7 @@ 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);
Expand Down Expand Up @@ -447,11 +448,9 @@ 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 inbounds GEPs with at most one variable offset.
auto IsNonConst = [](Value *V) { return !isa<ConstantInt>(V); };
if (!GEP->isInBounds() || count_if(GEP->indices(), IsNonConst) > 1)
return false;

if (!Explored.contains(GEP->getOperand(0)))
Expand Down Expand Up @@ -528,7 +527,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) {
Expand Down Expand Up @@ -559,29 +558,18 @@ 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");
}

auto *Op = NewInsts[GEP->getOperand(0)];
Value *Op = NewInsts[GEP->getOperand(0)];
Value *OffsetV = emitGEPOffset(&Builder, DL, GEP);
if (isa<ConstantInt>(Op) && cast<ConstantInt>(Op)->isZero())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: match(Op, m_Zero())

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))
Expand Down Expand Up @@ -609,23 +597,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.
Expand All @@ -635,32 +614,6 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, 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 *, Value *>
getAsConstantIndexedAddress(Type *ElemTy, Value *V, const DataLayout &DL) {
Type *IndexType = IntegerType::get(V->getContext(),
DL.getIndexTypeSizeInBits(V->getType()));

Constant *Index = ConstantInt::getNullValue(IndexType);
while (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
// We accept only inbouds GEPs here to exclude the possibility of
// overflow.
if (!GEP->isInBounds())
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;
}
return {V, Index};
}

/// 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.
Expand All @@ -675,14 +628,19 @@ 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);
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;

if (!canRewriteGEPAsOffset(ElemTy, RHS, PtrBase, DL, Nodes))
if (!canRewriteGEPAsOffset(RHS, PtrBase, DL, Nodes))
return nullptr;

// We know we can re-write this as
Expand All @@ -691,13 +649,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
Expand Down
45 changes: 25 additions & 20 deletions llvm/test/Transforms/InstCombine/indexed-gep-compares.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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@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:
Expand All @@ -34,15 +35,16 @@ bb2:
define ptr@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:
Expand Down Expand Up @@ -157,16 +159,17 @@ bb2:
define ptr@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:
Expand Down Expand Up @@ -195,14 +198,15 @@ define ptr@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 }
Expand Down Expand Up @@ -239,15 +243,16 @@ define ptr@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 }
Expand Down
21 changes: 11 additions & 10 deletions llvm/test/Transforms/InstCombine/opaque-ptr.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down