[VPlan] Add splitAt unit test. NFC #164636

lukel97 · 2025-10-22T14:43:53Z

@sink_replicate_region_4_requires_split_at_end_of_block was originally added to ensure splitting at the end of a block wouldn't crash, see bdada75

However it looks like we're now no longer testing this because conv isn't at the end of the block anymore.

This moves it into a unit test instead. Discovered when working on #160449

@sink_replicate_region_4_requires_split_at_end_of_block was originally added to ensure splitting at the end of a block wouldn't crash, see bdada75 However it looks like we're now no longer testing this because conv isn't at the end of the block anymore. This moves it into a unit test instead, which also prevents it from being disturbed by another PR: https://github.com/llvm/llvm-project/pull/160449/files/ac9bed0d6e35de2727af2891afe31333f5844930#r2451225426

llvmbot · 2025-10-22T14:44:40Z

@llvm/pr-subscribers-llvm-transforms

Author: Luke Lau (lukel97)

Changes

@sink_replicate_region_4_requires_split_at_end_of_block was originally added to ensure splitting at the end of a block wouldn't crash, see bdada75

However it looks like we're now no longer testing this because conv isn't at the end of the block anymore.

This moves it into a unit test instead, which also prevents it from being disturbed by another PR: https://github.com/llvm/llvm-project/pull/160449/files/ac9bed0d6e35de2727af2891afe31333f5844930#r2451225426

Full diff: https://github.com/llvm/llvm-project/pull/164636.diff

2 Files Affected:

(modified) llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll (-104)
(modified) llvm/unittests/Transforms/Vectorize/VPlanTest.cpp (+12)

diff --git a/llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll b/llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll
index 9deab9063d710..df69e53a64ca2 100644
--- a/llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll
+++ b/llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll
@@ -239,110 +239,6 @@ exit:
   ret i32 %res
 }
 
-; To sink the replicate region containing %rem, we need to split the block
-; containing %conv at the end, because %conv is the last recipe in the block.
-define void @sink_replicate_region_4_requires_split_at_end_of_block(i32 %x, ptr %ptr, ptr noalias %dst) optsize {
-; CHECK-LABEL: sink_replicate_region_4_requires_split_at_end_of_block
-; CHECK:      VPlan 'Initial VPlan for VF={2},UF>=1' {
-; CHECK-NEXT: Live-in vp<[[VF:%.+]]> = VF
-; CHECK-NEXT: Live-in vp<[[VFxUF:%.+]]> = VF * UF
-; CHECK-NEXT: Live-in vp<[[VEC_TC:%.+]]> = vector-trip-count
-; CHECK-NEXT: Live-in vp<[[BTC:%.+]]> = backedge-taken count
-; CHECK-NEXT: Live-in ir<20001> = original trip-count
-; CHECK-EMPTY:
-; CHECK-NEXT: ir-bb<entry>:
-; CHECK-NEXT: Successor(s): scalar.ph, vector.ph
-; CHECK-EMPTY:
-; CHECK-NEXT: vector.ph:
-; CHECK-NEXT: Successor(s): vector loop
-; CHECK-EMPTY:
-; CHECK-NEXT: <x1> vector loop: {
-; CHECK-NEXT: vector.body:
-; CHECK-NEXT:   EMIT vp<[[CAN_IV:%.+]]> = CANONICAL-INDUCTION
-; CHECK-NEXT:   FIRST-ORDER-RECURRENCE-PHI ir<%0> = phi ir<0>, ir<%conv>
-; CHECK-NEXT:   ir<%iv> = WIDEN-INDUCTION ir<0>, ir<1>, vp<[[VF]]>
-; CHECK-NEXT:   vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[CAN_IV]]>, ir<1>
-; CHECK-NEXT:   EMIT vp<[[MASK:%.+]]> = icmp ule ir<%iv>, vp<[[BTC]]>
-; CHECK-NEXT:   REPLICATE ir<%gep> = getelementptr ir<%ptr>, vp<[[STEPS]]>
-; CHECK-NEXT: Successor(s): pred.load
-; CHECK-EMPTY:
-; CHECK-NEXT: <xVFxUF> pred.load: {
-; CHECK-NEXT:   pred.load.entry:
-; CHECK-NEXT:     BRANCH-ON-MASK vp<[[MASK]]>
-; CHECK-NEXT:   Successor(s): pred.load.if, pred.load.continue
-; CHECK-EMPTY:
-; CHECK-NEXT:   pred.load.if:
-; CHECK-NEXT:     REPLICATE ir<%lv> = load ir<%gep> (S->V)
-; CHECK-NEXT:   Successor(s): pred.load.continue
-; CHECK-EMPTY:
-; CHECK-NEXT:   pred.load.continue:
-; CHECK-NEXT:     PHI-PREDICATED-INSTRUCTION vp<[[PRED:%.+]]> = ir<%lv>
-; CHECK-NEXT:   No successors
-; CHECK-NEXT: }
-; CHECK-NEXT: Successor(s): loop.0
-; CHECK-EMPTY:
-; CHECK-NEXT: loop.0:
-; CHECK-NEXT:   WIDEN-CAST ir<%conv> = sext vp<[[PRED]]> to i32
-; CHECK-NEXT:   EMIT vp<[[SPLICE:%.+]]> = first-order splice ir<%0>, ir<%conv>
-; CHECK-NEXT: Successor(s): pred.store
-; CHECK-EMPTY:
-; CHECK:      <xVFxUF> pred.store: {
-; CHECK-NEXT:   pred.store.entry:
-; CHECK-NEXT:     BRANCH-ON-MASK vp<[[MASK]]>
-; CHECK-NEXT:   Successor(s): pred.store.if, pred.store.continue
-; CHECK-EMPTY:
-; CHECK:        pred.store.if:
-; CHECK-NEXT:     REPLICATE ir<%lv.2> = load ir<%gep>
-; CHECK-NEXT:     REPLICATE ir<%rem> = srem vp<[[SPLICE]]>, ir<%x>
-; CHECK-NEXT:     REPLICATE ir<%conv.lv.2> = sext ir<%lv.2>
-; CHECK-NEXT:     REPLICATE ir<%add.1> = add ir<%conv>, ir<%rem>
-; CHECK-NEXT:     REPLICATE ir<%gep.dst> = getelementptr ir<%dst>, vp<[[STEPS]]>
-; CHECK-NEXT:     REPLICATE ir<%add> = add ir<%add.1>, ir<%conv.lv.2>
-; CHECK-NEXT:     REPLICATE store ir<%add>, ir<%gep.dst>
-; CHECK-NEXT:   Successor(s): pred.store.continue
-; CHECK-EMPTY:
-; CHECK:        pred.store.continue:
-; CHECK-NEXT:   No successors
-; CHECK-NEXT: }
-; CHECK-NEXT:   Successor(s): loop.2
-; CHECK-EMPTY:
-; CHECK:      loop.2:
-; CHECK-NEXT:   EMIT vp<[[CAN_IV_NEXT:%.+]]> = add nuw vp<[[CAN_IV]]>, vp<[[VFxUF]]>
-; CHECK-NEXT:   EMIT branch-on-count vp<[[CAN_IV_NEXT]]>, vp<[[VEC_TC]]>
-; CHECK-NEXT: No successors
-; CHECK-NEXT: }
-; CHECK-NEXT: Successor(s): middle.block
-; CHECK-EMPTY:
-; CHECK-NEXT: middle.block:
-; CHECK-NEXT: Successor(s): ir-bb<exit>
-; CHECK-EMPTY:
-; CHECK-NEXT: ir-bb<exit>
-; CHECK-NEXT: No successors
-;
-entry:
-  br label %loop
-
-loop:
-  %0 = phi i32 [ 0, %entry ], [ %conv, %loop ]
-  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
-  %gep = getelementptr i8, ptr %ptr, i32 %iv
-  %rem = srem i32 %0, %x
-  %lv = load i8, ptr %gep
-  %conv = sext i8 %lv to i32
-  %lv.2 = load i8, ptr %gep
-  %add.1 = add i32 %conv, %rem
-  %conv.lv.2 = sext i8 %lv.2 to i32
-  %add = add i32 %add.1, %conv.lv.2
-  %gep.dst = getelementptr i32, ptr %dst, i32 %iv
-  store i32 %add, ptr %gep.dst
-  %iv.next = add nsw i32 %iv, 1
-  %ec = icmp eq i32 %iv.next, 20001
-  br i1 %ec, label %exit, label %loop
-
-exit:
-  ret void
-}
-
 ; Test case that requires sinking a recipe in a replicate region after another replicate region.
 define void @sink_replicate_region_after_replicate_region(ptr %ptr, ptr noalias %dst.2, i32 %x, i8 %y) optsize {
 ; CHECK-LABEL: sink_replicate_region_after_replicate_region
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index db64c755d005f..7faea0ecc2f6d 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -704,6 +704,18 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
   }
 }
 
+TEST_F(VPBasicBlockTest, splitAtEnd) {
+  VPlan &Plan = getPlan();
+  VPInstruction *I1 = new VPInstruction(0, {});
+  VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("VPBB1", I1);
+  VPBlockUtils::connectBlocks(Plan.getEntry(), VPBB1);
+  VPBlockUtils::connectBlocks(VPBB1, Plan.getScalarHeader());
+  VPBB1->splitAt(VPBB1->end());
+  auto *Split = cast<VPBasicBlock>(VPBB1->getSingleSuccessor());
+  EXPECT_TRUE(Split->empty());
+  EXPECT_EQ(Split->getSingleSuccessor(), Plan.getScalarHeader());
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 TEST_F(VPBasicBlockTest, print) {
   VPInstruction *TC = new VPInstruction(Instruction::PHI, {});

lukel97 · 2025-10-30T16:11:26Z

Ping

lukel97 · 2025-11-06T14:19:45Z

Ping

artagnon

This is a good improvement!

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

…t-end-test

fhahn · 2025-11-06T14:35:59Z

llvm/test/Transforms/LoopVectorize/first-order-recurrence-sink-replicate-region.ll


-; To sink the replicate region containing %rem, we need to split the block
-; containing %conv at the end, because %conv is the last recipe in the block.
-define void @sink_replicate_region_4_requires_split_at_end_of_block(i32 %x, ptr %ptr, ptr noalias %dst) optsize {


Can/should we keep the test in addition with an explicit predicate, to keep testing the merge/sink logic as-is at the moment?

This test has the same issue as the sink_replicate_region_1 test above, where we can't predicate the srem that's being sunk because it's used in the FOR chain. I'll add the test back anyway.

Sounds good, thanks for checking

fhahn

LGTM, thanks

@sink_replicate_region_4_requires_split_at_end_of_block was originally added to ensure splitting at the end of a block wouldn't crash, see bdada75 However it looks like we're now no longer testing this because conv isn't at the end of the block anymore. This moves it into a unit test instead. Discovered when working on llvm#160449

lukel97 requested a review from fhahn October 22, 2025 14:43

llvmbot added the llvm:transforms label Oct 22, 2025

Merge branch 'main' into loop-vectorize/splitAt-end-test

3b1e6d2

lukel97 mentioned this pull request Nov 4, 2025

[VPlan] Don't apply predication discount to non-originally-predicated blocks #160449

Open

lukel97 requested a review from artagnon November 6, 2025 14:19

artagnon reviewed Nov 6, 2025

View reviewed changes

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp Show resolved Hide resolved

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into loop-vectorize/splitA…

7b6cbfc

…t-end-test

fhahn reviewed Nov 6, 2025

View reviewed changes

lukel97 added 2 commits November 6, 2025 22:38

Address review comments

3e6b2db

Add back LLVM IR test

6d9c63b

lukel97 changed the title ~~[VPlan] Move splitAt test to unit test. NFC~~ [VPlan] Add splitAt unit test. NFC Nov 6, 2025

fhahn approved these changes Nov 6, 2025

View reviewed changes

lukel97 enabled auto-merge (squash) November 6, 2025 14:52

lukel97 merged commit 3d589a9 into llvm:main Nov 6, 2025
9 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[VPlan] Add splitAt unit test. NFC #164636

[VPlan] Add splitAt unit test. NFC #164636

Uh oh!

lukel97 commented Oct 22, 2025 •

edited

Loading

Uh oh!

llvmbot commented Oct 22, 2025

Uh oh!

lukel97 commented Oct 30, 2025

Uh oh!

lukel97 commented Nov 6, 2025

Uh oh!

artagnon left a comment

Uh oh!

Uh oh!

Uh oh!

fhahn Nov 6, 2025

Uh oh!

lukel97 Nov 6, 2025

Uh oh!

fhahn Nov 6, 2025

Uh oh!

fhahn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[VPlan] Add splitAt unit test. NFC #164636

[VPlan] Add splitAt unit test. NFC #164636

Uh oh!

Conversation

lukel97 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 22, 2025

Uh oh!

lukel97 commented Oct 30, 2025

Uh oh!

lukel97 commented Nov 6, 2025

Uh oh!

artagnon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fhahn Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

lukel97 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lukel97 commented Oct 22, 2025 •

edited

Loading