This is an archive of the discontinued LLVM Phabricator instance.

Differential D149240

[InstCombine] Combine const GEP chains
ClosedPublic

Authored by dtcxzyw on Apr 26 2023, 12:07 AM.

Details

Reviewers
nikic
StephenFan
Commits
rG6d667d4b261e: [InstCombine] Combine const GEP chains
Summary

This patch reverts rGae739aefd7473517d3f08b5c8d08a66c7f469198 to address performance regressions reported by our CI after rG2ec1d0f427c7822540352c0c14d057e7bfe4f77b.

For example:

define ptr @const_gep_chain(ptr %p, i64 %a) {
    %p1 = getelementptr inbounds i8, ptr %p, i64 %a
    %p2 = getelementptr inbounds i8, ptr %p1, i64 1
    %p3 = getelementptr inbounds i8, ptr %p2, i64 2
    %p4 = getelementptr inbounds i8, ptr %p3, i64 3
    ret ptr %p4
}

The last three GEPs will not be folded since rG2ec1d0f427c7822540352c0c14d057e7bfe4f77b.

I think it is appropriate to remove this code because there is no compile-time regression reported in our benchmarks.

Diff Detail

Unit TestsFailed

TimeTest
270 msx64 debian > LLVM.Transforms/LoopVectorize::consecutive-ptr-uniforms.ll
100 msx64 debian > LLVM.Transforms/LoopVectorize/AArch64::vector-reverse-mask4.ll

Event Timeline

dtcxzyw created this revision.Apr 26 2023, 12:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2023, 12:07 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
dtcxzyw requested review of this revision.Apr 26 2023, 12:07 AM
Herald added subscribers: llvm-commits, • pcwang-thead. · View Herald TranscriptApr 26 2023, 12:07 AM
nikic accepted this revision.Apr 26 2023, 12:17 AM

LGTM. If we need to bring this check back, it will need a stronger condition.

This revision is now accepted and ready to land.Apr 26 2023, 12:17 AM
Harbormaster completed remote builds in B228231: Diff 517074.Apr 26 2023, 1:27 AM
dtcxzyw updated this revision to Diff 517215.Apr 26 2023, 9:54 AM

Update tests

nikic added inline comments.Apr 26 2023, 9:58 AM
llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll
1 ↗(On Diff #517215)

What's going on with the test changes in this file? I don't really get how this patch could cause them.

dtcxzyw updated this revision to Diff 517454.Apr 26 2023, 11:32 PM

Update tests

dtcxzyw added inline comments.Apr 26 2023, 11:34 PM
llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll
1 ↗(On Diff #517215)

Sorry, I just forgot to build the AArch64 target.

dtcxzyw marked an inline comment as done.Apr 26 2023, 11:35 PM
nikic added inline comments.Apr 27 2023, 12:08 AM
llvm/test/Transforms/LoopVectorize/consecutive-ptr-uniforms.ll
454 ↗(On Diff #517454)

Would it be enough to only update these two lines in this test, rather than delete the whole block?

dtcxzyw updated this revision to Diff 517622.Apr 27 2023, 10:04 AM

Update tests

dtcxzyw marked an inline comment as done.Apr 27 2023, 10:05 AM
nikic accepted this revision.Apr 27 2023, 12:23 PM

LGTM

nikic added a comment.Apr 30 2023, 7:43 AM

Do you have commit access? If not, can you let me know the Name <email> to use for the commit?

dtcxzyw updated this revision to Diff 518459.May 1 2023, 8:57 AM

Resolve conflicts before committing

This revision was landed with ongoing or failed builds.May 1 2023, 9:29 AM
Closed by commit rG6d667d4b261e: [InstCombine] Combine const GEP chains (authored by dtcxzyw). · Explain Why
This revision was automatically updated to reflect the committed changes.
dtcxzyw added a commit: rG6d667d4b261e: [InstCombine] Combine const GEP chains.
Harbormaster completed remote builds in B229248: Diff 518459.May 1 2023, 9:43 AM

Revision Contents

PathSize
llvm/
lib/
Transforms/
InstCombine/
7 lines
test/
Transforms/
InstCombine/
13 lines
LoopVectorize/
18 lines

Diff 517074

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Show First 20 Lines • Show All 1,939 Lines • ▼ Show 20 Lines
Instruction *InstCombinerImpl::visitGEPOfGEP(GetElementPtrInst &GEP, Instruction *InstCombinerImpl::visitGEPOfGEP(GetElementPtrInst &GEP,
GEPOperator *Src) { GEPOperator *Src) {
// Combine Indices - If the source pointer to this getelementptr instruction // Combine Indices - If the source pointer to this getelementptr instruction
// is a getelementptr instruction with matching element type, combine the // is a getelementptr instruction with matching element type, combine the
// indices of the two getelementptr instructions into a single instruction. // indices of the two getelementptr instructions into a single instruction.
if (!shouldMergeGEPs(*cast<GEPOperator>(&GEP), *Src)) if (!shouldMergeGEPs(*cast<GEPOperator>(&GEP), *Src))
return nullptr; return nullptr;
// Note that if our source is a gep chain itself then we wait for that
// chain to be resolved before we perform this transformation. This
// avoids us creating a TON of code in some cases.
if (auto *SrcGEP = dyn_cast<GEPOperator>(Src->getOperand(0)))
if (SrcGEP->getNumOperands() == 2 && shouldMergeGEPs(*Src, *SrcGEP))
return nullptr; // Wait until our source is folded to completion.
// For constant GEPs, use a more general offset-based folding approach. // For constant GEPs, use a more general offset-based folding approach.
Type *PtrTy = Src->getType()->getScalarType(); Type *PtrTy = Src->getType()->getScalarType();
if (GEP.hasAllConstantIndices() && if (GEP.hasAllConstantIndices() &&
(Src->hasOneUse() || Src->hasAllConstantIndices())) { (Src->hasOneUse() || Src->hasAllConstantIndices())) {
// Split Src into a variable part and a constant suffix. // Split Src into a variable part and a constant suffix.
gep_type_iterator GTI = gep_type_begin(*Src); gep_type_iterator GTI = gep_type_begin(*Src);
Type *BaseType = GTI.getIndexedType(); Type *BaseType = GTI.getIndexedType();
bool IsFirstType = true; bool IsFirstType = true;
▲ Show 20 Lines • Show All 2,422 Lines • Show Last 20 Lines

llvm/test/Transforms/InstCombine/getelementptr.ll

Show First 20 Lines • Show All 1,383 Lines • ▼ Show 20 Lines
define ptr @const_gep_0xi8_global() { define ptr @const_gep_0xi8_global() {
; CHECK-LABEL: @const_gep_0xi8_global( ; CHECK-LABEL: @const_gep_0xi8_global(
; CHECK-NEXT: ret ptr getelementptr ([0 x i8], ptr @g_0xi8_e, i64 0, i64 10) ; CHECK-NEXT: ret ptr getelementptr ([0 x i8], ptr @g_0xi8_e, i64 0, i64 10)
; ;
ret ptr getelementptr ([0 x i8], ptr @g_0xi8_e, i64 0, i64 10) ret ptr getelementptr ([0 x i8], ptr @g_0xi8_e, i64 0, i64 10)
} }
define ptr @const_gep_chain(ptr %p, i64 %a) {
; CHECK-LABEL: @const_gep_chain(
; CHECK-NEXT: [[P1:%.*]] = getelementptr inbounds i8, ptr [[P:%.*]], i64 [[A:%.*]]
; CHECK-NEXT: [[P4:%.*]] = getelementptr inbounds i8, ptr [[P1]], i64 6
; CHECK-NEXT: ret ptr [[P4]]
;
%p1 = getelementptr inbounds i8, ptr %p, i64 %a
%p2 = getelementptr inbounds i8, ptr %p1, i64 1
%p3 = getelementptr inbounds i8, ptr %p2, i64 2
%p4 = getelementptr inbounds i8, ptr %p3, i64 3
ret ptr %p4
}
!0 = !{!"branch_weights", i32 2, i32 10} !0 = !{!"branch_weights", i32 2, i32 10}

llvm/test/Transforms/LoopVectorize/interleaved-accesses.ll

Show First 20 Lines • Show All 739 Lines • ▼ Show 20 Lines
; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i32> [ <i32 0, i32 1, i32 2, i32 3>, [[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], [[VECTOR_BODY]] ] ; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i32> [ <i32 0, i32 1, i32 2, i32 3>, [[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], [[VECTOR_BODY]] ]
; CHECK-NEXT: [[TMP0:%.*]] = mul i64 [[INDEX]], 12 ; CHECK-NEXT: [[TMP0:%.*]] = mul i64 [[INDEX]], 12
; CHECK-NEXT: [[NEXT_GEP:%.*]] = getelementptr i8, ptr [[A:%.*]], i64 [[TMP0]] ; CHECK-NEXT: [[NEXT_GEP:%.*]] = getelementptr i8, ptr [[A:%.*]], i64 [[TMP0]]
; CHECK-NEXT: [[WIDE_VEC:%.*]] = load <12 x i32>, ptr [[NEXT_GEP]], align 4 ; CHECK-NEXT: [[WIDE_VEC:%.*]] = load <12 x i32>, ptr [[NEXT_GEP]], align 4
; CHECK-NEXT: [[STRIDED_VEC:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 0, i32 3, i32 6, i32 9> ; CHECK-NEXT: [[STRIDED_VEC:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 0, i32 3, i32 6, i32 9>
; CHECK-NEXT: [[STRIDED_VEC2:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 1, i32 4, i32 7, i32 10> ; CHECK-NEXT: [[STRIDED_VEC2:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 1, i32 4, i32 7, i32 10>
; CHECK-NEXT: [[STRIDED_VEC3:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 2, i32 5, i32 8, i32 11> ; CHECK-NEXT: [[STRIDED_VEC3:%.*]] = shufflevector <12 x i32> [[WIDE_VEC]], <12 x i32> poison, <4 x i32> <i32 2, i32 5, i32 8, i32 11>
; CHECK-NEXT: [[TMP1:%.*]] = add <4 x i32> [[STRIDED_VEC]], [[VEC_IND]] ; CHECK-NEXT: [[TMP1:%.*]] = add <4 x i32> [[STRIDED_VEC]], [[VEC_IND]]
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i32, ptr [[NEXT_GEP]], i64 2 ; CHECK-NEXT: [[TMP2:%.*]] = add <4 x i32> [[STRIDED_VEC2]], [[VEC_IND]]
; CHECK-NEXT: [[TMP3:%.*]] = add <4 x i32> [[STRIDED_VEC2]], [[VEC_IND]] ; CHECK-NEXT: [[TMP3:%.*]] = add <4 x i32> [[STRIDED_VEC3]], [[VEC_IND]]
; CHECK-NEXT: [[TMP4:%.*]] = add <4 x i32> [[STRIDED_VEC3]], [[VEC_IND]] ; CHECK-NEXT: [[TMP4:%.*]] = shufflevector <4 x i32> [[TMP1]], <4 x i32> [[TMP2]], <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
; CHECK-NEXT: [[TMP5:%.*]] = getelementptr inbounds i32, ptr [[TMP2]], i64 -2 ; CHECK-NEXT: [[TMP5:%.*]] = shufflevector <4 x i32> [[TMP3]], <4 x i32> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 undef, i32 undef, i32 undef, i32 undef>
; CHECK-NEXT: [[TMP6:%.*]] = shufflevector <4 x i32> [[TMP1]], <4 x i32> [[TMP3]], <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7> ; CHECK-NEXT: [[INTERLEAVED_VEC:%.*]] = shufflevector <8 x i32> [[TMP4]], <8 x i32> [[TMP5]], <12 x i32> <i32 0, i32 4, i32 8, i32 1, i32 5, i32 9, i32 2, i32 6, i32 10, i32 3, i32 7, i32 11>
; CHECK-NEXT: [[TMP7:%.*]] = shufflevector <4 x i32> [[TMP4]], <4 x i32> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 undef, i32 undef, i32 undef, i32 undef> ; CHECK-NEXT: store <12 x i32> [[INTERLEAVED_VEC]], ptr [[NEXT_GEP]], align 4
; CHECK-NEXT: [[INTERLEAVED_VEC:%.*]] = shufflevector <8 x i32> [[TMP6]], <8 x i32> [[TMP7]], <12 x i32> <i32 0, i32 4, i32 8, i32 1, i32 5, i32 9, i32 2, i32 6, i32 10, i32 3, i32 7, i32 11>
; CHECK-NEXT: store <12 x i32> [[INTERLEAVED_VEC]], ptr [[TMP5]], align 4
; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4 ; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
; CHECK-NEXT: [[VEC_IND_NEXT]] = add <4 x i32> [[VEC_IND]], <i32 4, i32 4, i32 4, i32 4> ; CHECK-NEXT: [[VEC_IND_NEXT]] = add <4 x i32> [[VEC_IND]], <i32 4, i32 4, i32 4, i32 4>
; CHECK-NEXT: [[TMP8:%.*]] = icmp eq i64 [[INDEX_NEXT]], 1024 ; CHECK-NEXT: [[TMP6:%.*]] = icmp eq i64 [[INDEX_NEXT]], 1024
; CHECK-NEXT: br i1 [[TMP8]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP20:![0-9]+]] ; CHECK-NEXT: br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP20:![0-9]+]]
; CHECK: middle.block: ; CHECK: middle.block:
; CHECK-NEXT: br i1 true, label [[FOR_COND_CLEANUP:%.*]], label [[SCALAR_PH]] ; CHECK-NEXT: br i1 true, label [[FOR_COND_CLEANUP:%.*]], label [[SCALAR_PH]]
; CHECK: scalar.ph: ; CHECK: scalar.ph:
; CHECK-NEXT: br label [[FOR_BODY:%.*]] ; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.cond.cleanup: ; CHECK: for.cond.cleanup:
; CHECK-NEXT: ret void ; CHECK-NEXT: ret void
; CHECK: for.body: ; CHECK: for.body:
; CHECK-NEXT: br i1 poison, label [[FOR_COND_CLEANUP]], label [[FOR_BODY]], !llvm.loop [[LOOP21:![0-9]+]] ; CHECK-NEXT: br i1 poison, label [[FOR_COND_CLEANUP]], label [[FOR_BODY]], !llvm.loop [[LOOP21:![0-9]+]]
▲ Show 20 Lines • Show All 815 Lines • Show Last 20 Lines