Skip to content

Commit 0abeb7b

Browse files
committed
[InstCombine] Improve inbounds preservation for ADD+GEP -> GEP+GEP
Given that we have a "add nuw" and a "getelementptr inbounds nuw" like this: %idx = add nuw i64 %idx1, %idx2 %gep = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx Then we can preserve the "inbounds nuw" flag when transforming that into two getelementptr instructions: %gep1 = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx1 %gep = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx2 Similarly for just having "nuw" instead of "inbounds nuw" on the getelementptr. Proof: https://alive2.llvm.org/ce/z/4uhfDq
1 parent 32e969b commit 0abeb7b

File tree

2 files changed

+30
-23
lines changed

2 files changed

+30
-23
lines changed

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3087,12 +3087,22 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
30873087
return nullptr;
30883088

30893089
if (GEP.getNumIndices() == 1) {
3090-
// We can only preserve inbounds if the original gep is inbounds, the add
3091-
// is nsw, and the add operands are non-negative.
3092-
auto CanPreserveInBounds = [&](bool AddIsNSW, Value *Idx1, Value *Idx2) {
3090+
auto CanPreserveNoWrapFlags = [&](bool AddIsNSW, bool AddIsNUW, Value *Idx1,
3091+
Value *Idx2) {
3092+
// Preserve "inbounds nuw" if the original gep is "inbounds nuw",
3093+
// and the add is "nuw".
3094+
if (GEP.isInBounds() && GEP.hasNoUnsignedWrap() && AddIsNUW)
3095+
return GEPNoWrapFlags::inBounds() | GEPNoWrapFlags::noUnsignedWrap();
3096+
// Preserve "inbounds" if the original gep is "inbounds", the add
3097+
// is "nsw", and the add operands are non-negative.
30933098
SimplifyQuery Q = SQ.getWithInstruction(&GEP);
3094-
return GEP.isInBounds() && AddIsNSW && isKnownNonNegative(Idx1, Q) &&
3095-
isKnownNonNegative(Idx2, Q);
3099+
if (GEP.isInBounds() && AddIsNSW && isKnownNonNegative(Idx1, Q) &&
3100+
isKnownNonNegative(Idx2, Q))
3101+
return GEPNoWrapFlags::inBounds();
3102+
// Preserve "nuw" if the original gep is "nuw", and the add is "nuw".
3103+
if (GEP.hasNoUnsignedWrap() && AddIsNUW)
3104+
return GEPNoWrapFlags::noUnsignedWrap();
3105+
return GEPNoWrapFlags::none();
30963106
};
30973107

30983108
// Try to replace ADD + GEP with GEP + GEP.
@@ -3104,15 +3114,15 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
31043114
// as:
31053115
// %newptr = getelementptr i32, ptr %ptr, i64 %idx1
31063116
// %newgep = getelementptr i32, ptr %newptr, i64 %idx2
3107-
bool IsInBounds = CanPreserveInBounds(
3108-
cast<OverflowingBinaryOperator>(GEP.getOperand(1))->hasNoSignedWrap(),
3109-
Idx1, Idx2);
3117+
bool NSW = match(GEP.getOperand(1), m_NSWAddLike(m_Value(), m_Value()));
3118+
bool NUW = match(GEP.getOperand(1), m_NUWAddLike(m_Value(), m_Value()));
3119+
GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(NSW, NUW, Idx1, Idx2);
31103120
auto *NewPtr =
31113121
Builder.CreateGEP(GEP.getSourceElementType(), GEP.getPointerOperand(),
3112-
Idx1, "", IsInBounds);
3113-
return replaceInstUsesWith(
3114-
GEP, Builder.CreateGEP(GEP.getSourceElementType(), NewPtr, Idx2, "",
3115-
IsInBounds));
3122+
Idx1, "", NWFlags);
3123+
return replaceInstUsesWith(GEP,
3124+
Builder.CreateGEP(GEP.getSourceElementType(),
3125+
NewPtr, Idx2, "", NWFlags));
31163126
}
31173127
ConstantInt *C;
31183128
if (match(GEP.getOperand(1), m_OneUse(m_SExtLike(m_OneUse(m_NSWAdd(
@@ -3123,17 +3133,16 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
31233133
// as:
31243134
// %newptr = getelementptr i32, ptr %ptr, i32 %idx1
31253135
// %newgep = getelementptr i32, ptr %newptr, i32 idx2
3126-
bool IsInBounds = CanPreserveInBounds(
3127-
/*IsNSW=*/true, Idx1, C);
3136+
GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(
3137+
/*IsNSW=*/true, /*IsNUW=*/false, Idx1, C);
31283138
auto *NewPtr = Builder.CreateGEP(
31293139
GEP.getSourceElementType(), GEP.getPointerOperand(),
3130-
Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "",
3131-
IsInBounds);
3140+
Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "", NWFlags);
31323141
return replaceInstUsesWith(
31333142
GEP,
31343143
Builder.CreateGEP(GEP.getSourceElementType(), NewPtr,
31353144
Builder.CreateSExt(C, GEP.getOperand(1)->getType()),
3136-
"", IsInBounds));
3145+
"", NWFlags));
31373146
}
31383147
}
31393148

llvm/test/Transforms/InstCombine/array.ll

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,25 +122,23 @@ define ptr @gep_inbounds_nuwaddlike(ptr %ptr, i64 %a, i64 %b) {
122122
ret ptr %gep
123123
}
124124

125-
; FIXME: Preserve "inbounds nuw".
126125
define ptr @gep_inbounds_add_nuw(ptr %ptr, i64 %a, i64 %b) {
127126
; CHECK-LABEL: define ptr @gep_inbounds_add_nuw(
128127
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
129-
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr i32, ptr [[PTR]], i64 [[A]]
130-
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i32, ptr [[TMP1]], i64 [[B]]
128+
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr inbounds nuw i32, ptr [[PTR]], i64 [[A]]
129+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds nuw i32, ptr [[TMP1]], i64 [[B]]
131130
; CHECK-NEXT: ret ptr [[GEP]]
132131
;
133132
%add = add nuw i64 %a, %b
134133
%gep = getelementptr inbounds nuw i32, ptr %ptr, i64 %add
135134
ret ptr %gep
136135
}
137136

138-
; FIXME: Preserve "nuw".
139137
define ptr @gep_add_nuw(ptr %ptr, i64 %a, i64 %b) {
140138
; CHECK-LABEL: define ptr @gep_add_nuw(
141139
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
142-
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr i32, ptr [[PTR]], i64 [[A]]
143-
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i32, ptr [[TMP1]], i64 [[B]]
140+
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr nuw i32, ptr [[PTR]], i64 [[A]]
141+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr nuw i32, ptr [[TMP1]], i64 [[B]]
144142
; CHECK-NEXT: ret ptr [[GEP]]
145143
;
146144
%add = add nuw i64 %a, %b

0 commit comments

Comments
 (0)