Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[VPlan] Connect Entry to scalar preheader during initial construction. #140132

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.

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
May 27, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
!fixup address latest comments, thanks
  • Loading branch information
fhahn committed May 27, 2025
commit f4a28d47881e1fd1c99fa73c21340a5cbda43b18
12 changes: 10 additions & 2 deletions 12 llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2366,6 +2366,8 @@ InnerLoopVectorizer::getOrCreateVectorTripCount(BasicBlock *InsertBlock) {
}

void InnerLoopVectorizer::introduceCheckBlockInVPlan(BasicBlock *CheckIRBB) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clarify that CheckBlock now excludes the initial trip-count check, which is expected to be already introduced before calling introduceCheckBlockInVPlan()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note, thanks

// Note: The block with the minimum trip-count check is already connected
// during earlier VPlan construction.
VPBlockBase *ScalarPH = Plan.getScalarPreheader();
VPBlockBase *PreVectorPH = VectorPHVPB->getSinglePredecessor();
assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors");
Expand Down Expand Up @@ -2463,6 +2465,10 @@ void InnerLoopVectorizer::emitIterationCountCheck(BasicBlock *Bypass) {
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);
LoopBypassBlocks.push_back(TCCheckBlock);

assert(cast<VPIRBasicBlock>(Plan.getEntry())->getIRBasicBlock() ==
TCCheckBlock &&
"Plan's entry must be TCCCheckBlock");
}

BasicBlock *InnerLoopVectorizer::emitSCEVChecks(BasicBlock *Bypass) {
Expand Down Expand Up @@ -7893,8 +7899,10 @@ EpilogueVectorizerMainLoop::emitIterationCountCheck(BasicBlock *Bypass,
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);

// Only generate add a new block for the trip-count check for the main loop.
// The epilogue will use the already existing VPlan entry block.
// When vectorizing the main loop, its trip-count check is placed in a new
// block, whereas the overall trip-count check is placed in the VPlan entry
// block. When vectorizing the epilogue loop, its trip-count check is placed
// in the VPlan entry block.
if (!ForEpilogue)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!ForEpilogue)
// Worth explaining?
if (!ForEpilogue)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added thanks

introduceCheckBlockInVPlan(TCCheckBlock);
return TCCheckBlock;
Expand Down
2 changes: 2 additions & 0 deletions 2 llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,8 @@ void VPlanTransforms::prepareForVectorization(
// with the middle block already connected to the exit block.
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
// Also connect the entry block to the scalar preheader.
// TODO: Also introduce a branch recipe together with the minimum trip count
// check.
VPBlockUtils::connectBlocks(Plan.getEntry(), ScalarPH);
Plan.getEntry()->swapSuccessors();

Expand Down
2 changes: 1 addition & 1 deletion 2 llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,7 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
using namespace llvm::VPlanPatternMatch;
for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
vp_depth_first_shallow(Plan.getEntry()))) {
if (VPBB->getNumSuccessors() != 2 || isa<VPIRBasicBlock>(VPBB) ||
if (VPBB->getNumSuccessors() != 2 || VPBB == Plan.getEntry() ||
!match(&VPBB->back(), m_BranchOnCond(m_True())))
continue;

Expand Down
20 changes: 16 additions & 4 deletions 20 llvm/test/Transforms/LoopVectorize/vplan-dot-printing.ll
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,28 @@ define void @print_call_and_memory(i64 %n, ptr noalias %y, ptr noalias %x) nounw
; CHECK-NEXT: "ir-bb\<for.body.preheader\>:\l" +
; CHECK-NEXT: "Successor(s): scalar.ph, vector.ph\l"
; CHECK-NEXT: ]
; CHECK-NEXT: N0 -> N1 [ label=""]
; CHECK-NEXT: N0 -> N1 [ label="T"]
; CHECK-NEXT: N0 -> N2 [ label="F"]
; CHECK-NEXT: N1 [label =
; CHECK-NEXT: "scalar.ph:\l" +
; CHECK-NEXT: " EMIT vp\<%bc.resume.val\> = resume-phi vp\<%2\>, ir\<0\>\l" +
; CHECK-NEXT: "Successor(s): ir-bb\<for.body\>\l"
; CHECK-NEXT: ]
; CHECK-NEXT: N1 -> N3 [ label=""]
; CHECK-NEXT: N3 [label =
; CHECK-NEXT: "ir-bb\<for.body\>:\l" +
; CHECK-NEXT: " IR %iv = phi i64 [ %iv.next, %for.body ], [ 0, %for.body.preheader ] (extra operand: vp\<%bc.resume.val\> from scalar.ph)\l" +
; CHECK: "No successors\l"
; CHECK-NEXT: ]
; CHECK-NEXT: N2 [label =
; CHECK-NEXT: "vector.ph:\l" +
; CHECK-NEXT: "Successor(s): vector loop\l"
; CHECK-NEXT: ]
; CHECK-NEXT: N1 -> N2 [ label="" lhead=cluster_N3]
; CHECK-NEXT: subgraph cluster_N3 {
; CHECK-NEXT: N2 -> N4 [ label="" lhead=cluster_N5]
; CHECK-NEXT: subgraph cluster_N5 {
; CHECK-NEXT: fontname=Courier
; CHECK-NEXT: label="\<x1\> vector loop"
; CHECK-NEXT: N2 [label =
; CHECK-NEXT: N4 [label =
; CHECK-NEXT: "vector.body:\l" +
; CHECK-NEXT: " EMIT vp\<[[CAN_IV:%.+]]\> = CANONICAL-INDUCTION ir\<0\>, vp\<[[CAN_IV_NEXT:%.+]]\>\l" +
; CHECK-NEXT: " vp\<[[STEPS:%.+]]\> = SCALAR-STEPS vp\<[[CAN_IV]]\>, ir\<1\>, vp\<[[VF]]\>\l" +
Expand Down
110 changes: 55 additions & 55 deletions 110 llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ TEST_F(VPlanHCFGTest, testBuildHCFGInnerLoop) {
auto Plan = buildVPlan(LoopHeader);

VPBasicBlock *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
EXPECT_EQ(0u, Entry->getNumPredecessors());
EXPECT_EQ(1u, Entry->getNumSuccessors());
EXPECT_EQ(2u, Entry->getNumSuccessors());

// Check that the region following the preheader consists of a block for the
// original header and a separate latch.
Expand Down Expand Up @@ -108,18 +107,35 @@ edge [fontname=Courier, fontsize=30]
compound=true
N0 [label =
"ir-bb\<entry\>:\l" +
"Successor(s): vector.ph\l"
"Successor(s): scalar.ph, vector.ph\l"
]
N0 -> N1 [ label=""]
N0 -> N1 [ label="T"]
N0 -> N2 [ label="F"]
N1 [label =
"scalar.ph:\l" +
"Successor(s): ir-bb\<for.body\>\l"
]
N1 -> N3 [ label=""]
N3 [label =
"ir-bb\<for.body\>:\l" +
" IR %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]\l" +
" IR %arr.idx = getelementptr inbounds i32, ptr %A, i64 %indvars.iv\l" +
" IR %l1 = load i32, ptr %arr.idx, align 4\l" +
" IR %res = add i32 %l1, 10\l" +
" IR store i32 %res, ptr %arr.idx, align 4\l" +
" IR %indvars.iv.next = add i64 %indvars.iv, 1\l" +
" IR %exitcond = icmp ne i64 %indvars.iv.next, %N\l" +
"No successors\l"
]
N2 [label =
"vector.ph:\l" +
"Successor(s): vector loop\l"
]
N1 -> N2 [ label="" lhead=cluster_N3]
subgraph cluster_N3 {
N2 -> N4 [ label="" lhead=cluster_N5]
subgraph cluster_N5 {
fontname=Courier
label="\<x1\> vector loop"
N2 [label =
N4 [label =
"vector.body:\l" +
" EMIT vp\<%2\> = CANONICAL-INDUCTION ir\<0\>, vp\<%index.next\>\l" +
" WIDEN-PHI ir\<%indvars.iv\> = phi [ ir\<0\>, vector.ph ], [ ir\<%indvars.iv.next\>, vector.body ]\l" +
Expand All @@ -135,33 +151,17 @@ compound=true
"No successors\l"
]
}
N2 -> N4 [ label="" ltail=cluster_N3]
N4 [label =
N4 -> N6 [ label="" ltail=cluster_N5]
N6 [label =
"middle.block:\l" +
" EMIT vp\<%cmp.n\> = icmp eq ir\<%N\>, vp\<%1\>\l" +
" EMIT branch-on-cond vp\<%cmp.n\>\l" +
"Successor(s): ir-bb\<for.end\>, scalar.ph\l"
]
N4 -> N5 [ label="T"]
N4 -> N6 [ label="F"]
N5 [label =
"ir-bb\<for.end\>:\l" +
"No successors\l"
]
N6 [label =
"scalar.ph:\l" +
"Successor(s): ir-bb\<for.body\>\l"
]
N6 -> N7 [ label=""]
N6 -> N7 [ label="T"]
N6 -> N1 [ label="F"]
N7 [label =
"ir-bb\<for.body\>:\l" +
" IR %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]\l" +
" IR %arr.idx = getelementptr inbounds i32, ptr %A, i64 %indvars.iv\l" +
" IR %l1 = load i32, ptr %arr.idx, align 4\l" +
" IR %res = add i32 %l1, 10\l" +
" IR store i32 %res, ptr %arr.idx, align 4\l" +
" IR %indvars.iv.next = add i64 %indvars.iv, 1\l" +
" IR %exitcond = icmp ne i64 %indvars.iv.next, %N\l" +
"ir-bb\<for.end\>:\l" +
"No successors\l"
]
}
Expand Down Expand Up @@ -206,9 +206,8 @@ TEST_F(VPlanHCFGTest, testVPInstructionToVPRecipesInner) {
Plan, [](PHINode *P) { return nullptr; }, *SE, TLI);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
EXPECT_EQ(0u, Entry->getNumPredecessors());
EXPECT_EQ(1u, Entry->getNumSuccessors());
EXPECT_EQ(2u, Entry->getNumSuccessors());

// Check that the region following the preheader consists of a block for the
// original header and a separate latch.
Expand Down Expand Up @@ -277,18 +276,32 @@ edge [fontname=Courier, fontsize=30]
compound=true
N0 [label =
"ir-bb\<entry\>:\l" +
"Successor(s): vector.ph\l"
"Successor(s): scalar.ph, vector.ph\l"
]
N0 -> N1 [ label=""]
N0 -> N1 [ label="T"]
N0 -> N2 [ label="F"]
N1 [label =
"scalar.ph:\l" +
"Successor(s): ir-bb\<loop.header\>\l"
]
N1 -> N3 [ label=""]
N3 [label =
"ir-bb\<loop.header\>:\l" +
" IR %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop.latch ]\l" +
" IR %arr.idx = getelementptr inbounds i32, ptr %A, i64 %iv\l" +
" IR %l1 = load i32, ptr %arr.idx, align 4\l" +
" IR %c = icmp eq i32 %l1, 0\l" +
"No successors\l"
]
N2 [label =
"vector.ph:\l" +
"Successor(s): vector loop\l"
]
N1 -> N2 [ label="" lhead=cluster_N3]
subgraph cluster_N3 {
N2 -> N4 [ label="" lhead=cluster_N5]
subgraph cluster_N5 {
fontname=Courier
label="\<x1\> vector loop"
N2 [label =
N4 [label =
"vector.body:\l" +
" EMIT vp\<%2\> = CANONICAL-INDUCTION ir\<0\>, vp\<%index.next\>\l" +
" WIDEN-PHI ir\<%iv\> = phi [ ir\<0\>, vector.ph ], [ ir\<%iv.next\>, loop.latch ]\l" +
Expand All @@ -297,8 +310,8 @@ compound=true
" EMIT ir\<%c\> = icmp ir\<%l1\>, ir\<0\>\l" +
"Successor(s): loop.latch\l"
]
N2 -> N4 [ label=""]
N4 [label =
N4 -> N6 [ label=""]
N6 [label =
"loop.latch:\l" +
" EMIT ir\<%res\> = add ir\<%l1\>, ir\<10\>\l" +
" EMIT store ir\<%res\>, ir\<%arr.idx\>\l" +
Expand All @@ -310,30 +323,17 @@ compound=true
"No successors\l"
]
}
N4 -> N5 [ label="" ltail=cluster_N3]
N5 [label =
N6 -> N7 [ label="" ltail=cluster_N5]
N7 [label =
"middle.block:\l" +
" EMIT vp\<%cmp.n\> = icmp eq ir\<%N\>, vp\<%1\>\l" +
" EMIT branch-on-cond vp\<%cmp.n\>\l" +
"Successor(s): ir-bb\<exit.2\>, scalar.ph\l"
]
N5 -> N6 [ label="T"]
N5 -> N7 [ label="F"]
N6 [label =
"ir-bb\<exit.2\>:\l" +
"No successors\l"
]
N7 [label =
"scalar.ph:\l" +
"Successor(s): ir-bb\<loop.header\>\l"
]
N7 -> N8 [ label=""]
N7 -> N8 [ label="T"]
N7 -> N1 [ label="F"]
N8 [label =
"ir-bb\<loop.header\>:\l" +
" IR %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop.latch ]\l" +
" IR %arr.idx = getelementptr inbounds i32, ptr %A, i64 %iv\l" +
" IR %l1 = load i32, ptr %arr.idx, align 4\l" +
" IR %c = icmp eq i32 %l1, 0\l" +
"ir-bb\<exit.2\>:\l" +
"No successors\l"
]
}
Expand Down
24 changes: 0 additions & 24 deletions 24 llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ TEST_F(VPlanSlpTest, testSlpSimple_2) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 13));
Expand Down Expand Up @@ -169,8 +167,6 @@ TEST_F(VPlanSlpTest, testSlpSimple_3) {
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
auto Plan = buildVPlan(LoopHeader);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 13));
Expand Down Expand Up @@ -241,8 +237,6 @@ TEST_F(VPlanSlpTest, testSlpReuse_1) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 9));
Expand Down Expand Up @@ -305,8 +299,6 @@ TEST_F(VPlanSlpTest, testSlpReuse_2) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 6));
Expand Down Expand Up @@ -442,8 +434,6 @@ TEST_F(VPlanSlpTest, testSlpReorder_1) {
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
auto Plan = buildVPlan(LoopHeader);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 25));
Expand Down Expand Up @@ -514,8 +504,6 @@ TEST_F(VPlanSlpTest, testSlpReorder_2) {
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
auto Plan = buildVPlan(LoopHeader);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 25));
Expand Down Expand Up @@ -586,8 +574,6 @@ TEST_F(VPlanSlpTest, testSlpReorder_3) {
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
auto Plan = buildVPlan(LoopHeader);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 25));
Expand Down Expand Up @@ -662,8 +648,6 @@ TEST_F(VPlanSlpTest, testSlpReorder_4) {
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
auto Plan = buildVPlan(LoopHeader);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 25));
Expand Down Expand Up @@ -723,8 +707,6 @@ TEST_F(VPlanSlpTest, testInstrsInDifferentBBs) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();
VPBasicBlock *BB2 = Body->getSingleSuccessor()->getEntryBasicBlock();

Expand Down Expand Up @@ -786,8 +768,6 @@ TEST_F(VPlanSlpTest, testInstrsInDifferentBBs2) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();
VPBasicBlock *BB2 = Body->getSingleSuccessor()->getEntryBasicBlock();

Expand Down Expand Up @@ -846,8 +826,6 @@ TEST_F(VPlanSlpTest, testSlpAtomicLoad) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 13));
Expand Down Expand Up @@ -905,8 +883,6 @@ TEST_F(VPlanSlpTest, testSlpAtomicStore) {
auto Plan = buildVPlan(LoopHeader);
auto VPIAI = getInterleavedAccessInfo(*F, LI->getLoopFor(LoopHeader), *Plan);

VPBlockBase *Entry = Plan->getEntry()->getEntryBasicBlock();
EXPECT_NE(nullptr, Entry->getSingleSuccessor());
VPBasicBlock *Body = Plan->getVectorLoopRegion()->getEntryBasicBlock();

VPInstruction *Store1 = cast<VPInstruction>(&*std::next(Body->begin(), 13));
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.