[VPlan] Remove CanonicalIV when dissolving loop regions (NFC). by fhahn · Pull Request #142372 · llvm/llvm-project

fhahn · 2025-06-02T12:22:17Z

Directly replace the canonical IV when we dissolve the containing region. That ensures that it won't get removed before the region gets removed, which would result in an invalid region.

This removes the current ordering constraint between convertToConcreteRecipes and dissolving regions.

Directly replace the canonical IV when we dissolve the containing region. That ensures that it won't get removed before the region gets removed, which would result in an invalid region. This removes the current ordering constraint between convertToConcreteRecipes and dissolving regions.

llvmbot · 2025-06-02T12:23:03Z

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Directly replace the canonical IV when we dissolve the containing region. That ensures that it won't get removed before the region gets removed, which would result in an invalid region.

This removes the current ordering constraint between convertToConcreteRecipes and dissolving regions.

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

2 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (+10)
(modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+3-7)

diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 165b57c87beb1..2580031c9186b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -883,6 +883,16 @@ void VPRegionBlock::print(raw_ostream &O, const Twine &Indent,
 
 void VPRegionBlock::dissolveToCFGLoop() {
   auto *Header = cast<VPBasicBlock>(getEntry());
+  if (auto *CanIV = dyn_cast<VPCanonicalIVPHIRecipe>(&Header->front())) {
+    assert(this == getPlan()->getVectorLoopRegion() &&
+           "Canonical IV must be in the entry of the top-level loop region");
+    auto *ScalarR = Builder(CanIV).createScalarPhi(
+        {CanIV->getStartValue(), CanIV->getBackedgeValue()},
+        CanIV->getDebugLoc(), "index");
+    CanIV->replaceAllUsesWith(ScalarR);
+    CanIV->eraseFromParent();
+  }
+
   VPBlockBase *Preheader = getSinglePredecessor();
   auto *ExitingLatch = cast<VPBasicBlock>(getExiting());
   VPBlockBase *Middle = getSingleSuccessor();
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 5b42a9056b69e..5c6d7b0c5b529 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -2601,14 +2601,10 @@ void VPlanTransforms::convertToConcreteRecipes(VPlan &Plan,
   for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
            vp_depth_first_deep(Plan.getEntry()))) {
     for (VPRecipeBase &R : make_early_inc_range(*VPBB)) {
-      if (isa<VPCanonicalIVPHIRecipe, VPEVLBasedIVPHIRecipe>(&R)) {
-        auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
-        StringRef Name =
-            isa<VPCanonicalIVPHIRecipe>(PhiR) ? "index" : "evl.based.iv";
-        VPBuilder Builder(PhiR);
-        auto *ScalarR = Builder.createScalarPhi(
+      if (auto *PhiR = dyn_cast<VPEVLBasedIVPHIRecipe>(&R)) {
+        auto *ScalarR = Builder(PhiR).createScalarPhi(
             {PhiR->getStartValue(), PhiR->getBackedgeValue()},
-            PhiR->getDebugLoc(), Name);
+            PhiR->getDebugLoc(), "evl.based.iv");
         PhiR->replaceAllUsesWith(ScalarR);
         ToRemove.push_back(PhiR);
         continue;

lukel97

Makes sense, LGTM

lukel97 · 2025-06-02T12:39:48Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

 void VPRegionBlock::dissolveToCFGLoop() {
  auto *Header = cast<VPBasicBlock>(getEntry());
+  if (auto *CanIV = dyn_cast<VPCanonicalIVPHIRecipe>(&Header->front())) {
+    assert(this == getPlan()->getVectorLoopRegion() &&


Just an idea, alternatively it could also be replaced in VPlanTransforms::dissolveLoopRegions where you could get it directly from getCanonicalIV()?

Yep that would also do the trick, but doing it here has the advantage it cannot be missed by potential other users

david-arm · 2025-06-02T12:47:43Z

Looks like there are build failures:

C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(889): error C3861: 'Builder': identifier not found
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): error C3536: 'ScalarR': cannot be used before it is initialized
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): error C2664: 'void llvm::VPValue::replaceAllUsesWith(llvm::VPValue *)': cannot convert argument 1 from 'int' to 'llvm::VPValue *'
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): note: Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlanValue.h(156): note: see declaration of 'llvm::VPValue::replaceAllUsesWith'

…hen-dissolving-region

fhahn

Looks like there are build failures:

C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(889): error C3861: 'Builder': identifier not found
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): error C3536: 'ScalarR': cannot be used before it is initialized
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): error C2664: 'void llvm::VPValue::replaceAllUsesWith(llvm::VPValue *)': cannot convert argument 1 from 'int' to 'llvm::VPValue *'
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlan.cpp(892): note: Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast
C:\ws\src\llvm\lib\Transforms\Vectorize\VPlanValue.h(156): note: see declaration of 'llvm::VPValue::replaceAllUsesWith'

Thanks, windows build failure should be fixed

fhahn · 2025-06-03T07:50:32Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

 void VPRegionBlock::dissolveToCFGLoop() {
  auto *Header = cast<VPBasicBlock>(getEntry());
+  if (auto *CanIV = dyn_cast<VPCanonicalIVPHIRecipe>(&Header->front())) {
+    assert(this == getPlan()->getVectorLoopRegion() &&


Yep that would also do the trick, but doing it here has the advantage it cannot be missed by potential other users

…FC). (#142372) Directly replace the canonical IV when we dissolve the containing region. That ensures that it won't get removed before the region gets removed, which would result in an invalid region. This removes the current ordering constraint between convertToConcreteRecipes and dissolving regions. PR: llvm/llvm-project#142372

ayalz · 2025-06-03T11:30:09Z

Post-commit follow-up comment: this raises the thought of coupling CanonicalIVPHI with its loop Region - also to be created together, i.e., within a createFromCFGLoop() complementing dissolveToCFGLoop(). A loop region could hold its CanonicalIVPHI rather than place it inside its header block, abstracting away CanonicalIVIncrement along with its cyclic dependence w/o risk of dce, guaranteeing CanonicalIVPHI's uniqueness.

fhahn requested review from aniragil, ayalz, lukel97 and rengolin June 2, 2025 12:22

llvmbot added vectorizers llvm:transforms labels Jun 2, 2025

fhahn mentioned this pull request Jun 2, 2025

[VPlan] Convert to concrete recipes before dissolving loop regions. NFCI #141999

Merged

lukel97 approved these changes Jun 2, 2025

View reviewed changes

fhahn added 2 commits June 3, 2025 08:40

Merge remote-tracking branch 'origin/main' into vplan-replace-caniv-w…

cd93b33

…hen-dissolving-region

!fixup fix build failure.

95b2d84

fhahn commented Jun 3, 2025

View reviewed changes

fhahn merged commit 2eab83f into llvm:main Jun 3, 2025
11 checks passed

fhahn deleted the vplan-replace-caniv-when-dissolving-region branch June 3, 2025 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VPlan] Remove CanonicalIV when dissolving loop regions (NFC).#142372

[VPlan] Remove CanonicalIV when dissolving loop regions (NFC).#142372
fhahn merged 3 commits intollvm:mainfrom
fhahn:vplan-replace-caniv-when-dissolving-region

fhahn commented Jun 2, 2025

Uh oh!

llvmbot commented Jun 2, 2025 •

edited

Loading

Uh oh!

lukel97 left a comment

Uh oh!

lukel97 Jun 2, 2025

Uh oh!

fhahn Jun 3, 2025

Uh oh!

david-arm commented Jun 2, 2025

Uh oh!

fhahn left a comment

Uh oh!

fhahn Jun 3, 2025

Uh oh!

Uh oh!

ayalz commented Jun 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

fhahn commented Jun 2, 2025

Uh oh!

llvmbot commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukel97 left a comment

Choose a reason for hiding this comment

Uh oh!

lukel97 Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

david-arm commented Jun 2, 2025

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

fhahn Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ayalz commented Jun 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

llvmbot commented Jun 2, 2025 •

edited

Loading