Skip to content

Commit 567c02a

Browse files
committed
[InstCombine] Remove inttoptr/ptrtoint handling from indexed compare fold
Looking through inttoptr / ptrtoint intermixed with GEPs is very questionable from a provenance perspective. We also don't seem to have any test coverage that shows this is useful (apart from one test I added to guard against a crash).
1 parent d5f3b3b commit 567c02a

File tree

2 files changed

+30
-57
lines changed

2 files changed

+30
-57
lines changed

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -441,21 +441,11 @@ static bool canRewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
441441
continue;
442442
}
443443

444-
if (!isa<IntToPtrInst>(V) && !isa<PtrToIntInst>(V) &&
445-
!isa<GetElementPtrInst>(V) && !isa<PHINode>(V))
444+
if (!isa<GetElementPtrInst>(V) && !isa<PHINode>(V))
446445
// We've found some value that we can't explore which is different from
447446
// the base. Therefore we can't do this transformation.
448447
return false;
449448

450-
if (isa<IntToPtrInst>(V) || isa<PtrToIntInst>(V)) {
451-
auto *CI = cast<CastInst>(V);
452-
if (!CI->isNoopCast(DL))
453-
return false;
454-
455-
if (!Explored.contains(CI->getOperand(0)))
456-
WorkList.push_back(CI->getOperand(0));
457-
}
458-
459449
if (auto *GEP = dyn_cast<GEPOperator>(V)) {
460450
// We're limiting the GEP to having one index. This will preserve
461451
// the original pointer type. We could handle more cases in the
@@ -573,13 +563,6 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
573563
if (NewInsts.contains(Val))
574564
continue;
575565

576-
if (auto *CI = dyn_cast<CastInst>(Val)) {
577-
// Don't get rid of the intermediate variable here; the store can grow
578-
// the map which will invalidate the reference to the input value.
579-
Value *V = NewInsts[CI->getOperand(0)];
580-
NewInsts[CI] = V;
581-
continue;
582-
}
583566
if (auto *GEP = dyn_cast<GEPOperator>(Val)) {
584567
Value *Index = NewInsts[GEP->getOperand(1)] ? NewInsts[GEP->getOperand(1)]
585568
: GEP->getOperand(1);
@@ -652,41 +635,25 @@ static Value *rewriteGEPAsOffset(Type *ElemTy, Value *Start, Value *Base,
652635
return NewInsts[Start];
653636
}
654637

655-
/// Looks through GEPs, IntToPtrInsts and PtrToIntInsts in order to express
656-
/// the input Value as a constant indexed GEP. Returns a pair containing
657-
/// the GEPs Pointer and Index.
638+
/// Looks through GEPs in order to express the input Value as a constant
639+
/// indexed GEP. Returns a pair containing the GEPs Pointer and Index.
658640
static std::pair<Value *, Value *>
659641
getAsConstantIndexedAddress(Type *ElemTy, Value *V, const DataLayout &DL) {
660642
Type *IndexType = IntegerType::get(V->getContext(),
661643
DL.getIndexTypeSizeInBits(V->getType()));
662644

663645
Constant *Index = ConstantInt::getNullValue(IndexType);
664-
while (true) {
665-
if (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
666-
// We accept only inbouds GEPs here to exclude the possibility of
667-
// overflow.
668-
if (!GEP->isInBounds())
669-
break;
670-
if (GEP->hasAllConstantIndices() && GEP->getNumIndices() == 1 &&
671-
GEP->getSourceElementType() == ElemTy &&
672-
GEP->getOperand(1)->getType() == IndexType) {
673-
V = GEP->getOperand(0);
674-
Constant *GEPIndex = static_cast<Constant *>(GEP->getOperand(1));
675-
Index = ConstantExpr::getAdd(Index, GEPIndex);
676-
continue;
677-
}
646+
while (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
647+
// We accept only inbouds GEPs here to exclude the possibility of
648+
// overflow.
649+
if (!GEP->isInBounds())
678650
break;
679-
}
680-
if (auto *CI = dyn_cast<IntToPtrInst>(V)) {
681-
if (!CI->isNoopCast(DL))
682-
break;
683-
V = CI->getOperand(0);
684-
continue;
685-
}
686-
if (auto *CI = dyn_cast<PtrToIntInst>(V)) {
687-
if (!CI->isNoopCast(DL))
688-
break;
689-
V = CI->getOperand(0);
651+
if (GEP->hasAllConstantIndices() && GEP->getNumIndices() == 1 &&
652+
GEP->getSourceElementType() == ElemTy &&
653+
GEP->getOperand(1)->getType() == IndexType) {
654+
V = GEP->getOperand(0);
655+
Constant *GEPIndex = static_cast<Constant *>(GEP->getOperand(1));
656+
Index = ConstantExpr::getAdd(Index, GEPIndex);
690657
continue;
691658
}
692659
break;

llvm/test/Transforms/InstCombine/indexed-gep-compares.ll

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ define ptr@test2(i32 %A, i32 %Offset) {
4141
; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
4242
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
4343
; CHECK: bb2:
44-
; CHECK-NEXT: [[RHSTO_PTR:%.*]] = inttoptr i32 [[A:%.*]] to ptr
45-
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[RHSTO_PTR]], i32 [[RHS_IDX]]
44+
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[A:%.*]] to ptr
45+
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
4646
; CHECK-NEXT: ret ptr [[RHS_PTR]]
4747
;
4848
entry:
@@ -107,8 +107,8 @@ define ptr@test4(i16 %A, i32 %Offset) {
107107
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
108108
; CHECK: bb2:
109109
; CHECK-NEXT: [[TMP0:%.*]] = zext i16 [[A:%.*]] to i32
110-
; CHECK-NEXT: [[RHSTO_PTR:%.*]] = inttoptr i32 [[TMP0]] to ptr
111-
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[RHSTO_PTR]], i32 [[RHS_IDX]]
110+
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[TMP0]] to ptr
111+
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
112112
; CHECK-NEXT: ret ptr [[RHS_PTR]]
113113
;
114114
entry:
@@ -135,7 +135,7 @@ define ptr@test5(i32 %Offset) personality ptr @__gxx_personality_v0 {
135135
; CHECK-LABEL: @test5(
136136
; CHECK-NEXT: entry:
137137
; CHECK-NEXT: [[A:%.*]] = invoke ptr @fun_ptr()
138-
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
138+
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
139139
; CHECK: cont:
140140
; CHECK-NEXT: br label [[BB:%.*]]
141141
; CHECK: bb:
@@ -148,7 +148,7 @@ define ptr@test5(i32 %Offset) personality ptr @__gxx_personality_v0 {
148148
; CHECK-NEXT: ret ptr [[RHS_PTR]]
149149
; CHECK: lpad:
150150
; CHECK-NEXT: [[L:%.*]] = landingpad { ptr, i32 }
151-
; CHECK-NEXT: cleanup
151+
; CHECK-NEXT: cleanup
152152
; CHECK-NEXT: ret ptr null
153153
;
154154
entry:
@@ -179,7 +179,7 @@ define ptr@test6(i32 %Offset) personality ptr @__gxx_personality_v0 {
179179
; CHECK-LABEL: @test6(
180180
; CHECK-NEXT: entry:
181181
; CHECK-NEXT: [[A:%.*]] = invoke i32 @fun_i32()
182-
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
182+
; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
183183
; CHECK: cont:
184184
; CHECK-NEXT: br label [[BB:%.*]]
185185
; CHECK: bb:
@@ -188,12 +188,12 @@ define ptr@test6(i32 %Offset) personality ptr @__gxx_personality_v0 {
188188
; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[RHS_IDX]], 100
189189
; CHECK-NEXT: br i1 [[COND]], label [[BB2:%.*]], label [[BB]]
190190
; CHECK: bb2:
191-
; CHECK-NEXT: [[RHSTO_PTR:%.*]] = inttoptr i32 [[A]] to ptr
192-
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[RHSTO_PTR]], i32 [[RHS_IDX]]
191+
; CHECK-NEXT: [[A_PTR:%.*]] = inttoptr i32 [[A]] to ptr
192+
; CHECK-NEXT: [[RHS_PTR:%.*]] = getelementptr inbounds i32, ptr [[A_PTR]], i32 [[RHS_IDX]]
193193
; CHECK-NEXT: ret ptr [[RHS_PTR]]
194194
; CHECK: lpad:
195195
; CHECK-NEXT: [[L:%.*]] = landingpad { ptr, i32 }
196-
; CHECK-NEXT: cleanup
196+
; CHECK-NEXT: cleanup
197197
; CHECK-NEXT: ret ptr null
198198
;
199199
entry:
@@ -272,10 +272,16 @@ entry:
272272
define void @test_zero_offset_cycle(ptr %arg) {
273273
; CHECK-LABEL: @test_zero_offset_cycle(
274274
; CHECK-NEXT: entry:
275+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds { i64, i64 }, ptr [[ARG:%.*]], i32 0, i32 1
276+
; CHECK-NEXT: [[GEP_INT:%.*]] = ptrtoint ptr [[GEP]] to i32
275277
; CHECK-NEXT: br label [[LOOP:%.*]]
276278
; CHECK: loop:
277-
; CHECK-NEXT: br i1 true, label [[LOOP]], label [[LOOP_CONT:%.*]]
279+
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[GEP_INT]], [[ENTRY:%.*]] ], [ [[GEP_INT2:%.*]], [[LOOP_CONT:%.*]] ], [ [[PHI]], [[LOOP]] ]
280+
; CHECK-NEXT: [[PHI_PTR:%.*]] = inttoptr i32 [[PHI]] to ptr
281+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[GEP]], [[PHI_PTR]]
282+
; CHECK-NEXT: br i1 [[CMP]], label [[LOOP]], label [[LOOP_CONT]]
278283
; CHECK: loop.cont:
284+
; CHECK-NEXT: [[GEP_INT2]] = ptrtoint ptr [[GEP]] to i32
279285
; CHECK-NEXT: br label [[LOOP]]
280286
;
281287
entry:

0 commit comments

Comments
 (0)