[llvm] [VPlan] Verify dominance for incoming values of phi-like recipes. (PR #124838)

[Florian Hahn via llvm-commits]

https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/124838

>From bae5c8354faf01f7185e4eebbebc83614d1587f9 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 28 Jan 2025 20:35:02 +0000
Subject: [PATCH 1/2] [VPlan] Verify dominance for incoming values of phi-like
 recipes.

Update the verifier to verify dominance for incoming values for phi-like
recipes. The defining recipe must dominate the incoming block
for the incoming value.

There are 4 different cases to consider when retrieving the incoming
block:
  * VPIRInstructions wrapping phis: the incoming block from the blocks
    predecessors has the same index as the incoming value operand.
  * VPHeaderPhiRecipes, VPWidenPhi used in header: there's no
    predecessors; if the incoming value is the start value use the
    predecessor of the containing region, otherwise the exiting block of
    the region
 * 1 predecessor: Must be a VPWidenPHI used as LCSSA phi; use the
   predecessor
 * 2 predecessors: must be a VPPredInstPhiRecipe, use the second
   predecessor.
---
 .../Transforms/Vectorize/VPlanVerifier.cpp    | 58 +++++++++++++++++--
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index 0f151c897d938e..0948452e8d19cf 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -175,6 +175,11 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
   });
 }
 
+/// Return true if \p R is a VPIRInstruction wrapping a phi.
+static bool isVPIRInstructionPhi(const VPRecipeBase &R) {
+  auto *VPIRI = dyn_cast<VPIRInstruction>(&R);
+  return VPIRI && isa<PHINode>(VPIRI->getInstruction());
+}
 bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
   if (!verifyPhiRecipes(VPBB))
     return false;
@@ -207,14 +212,57 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
 
       for (const VPUser *U : V->users()) {
         auto *UI = cast<VPRecipeBase>(U);
-        // TODO: check dominance of incoming values for phis properly.
-        if (!UI ||
-            isa<VPHeaderPHIRecipe, VPWidenPHIRecipe, VPPredInstPHIRecipe>(UI))
+        const VPBlockBase *UserVPBB = UI->getParent();
+
+        // Verify incoming values of VPIRInstructions wrapping phis. V most
+        // dominate the end of the incoming block. The operand index of the
+        // incoming value matches the predecessor block index of the
+        // corresponding incoming block.
+        if (isVPIRInstructionPhi(*UI)) {
+          for (const auto &[Idx, Op] : enumerate(UI->operands())) {
+            if (V != Op)
+              continue;
+            const VPBlockBase *Incoming = UserVPBB->getPredecessors()[Idx];
+            if (Incoming != VPBB && !VPDT.dominates(VPBB, Incoming)) {
+              errs() << "Use before def!\n";
+              return false;
+            }
+          }
           continue;
+        }
+        // Verify incoming value of various phi-like recipes.
+        if (isa<VPWidenPHIRecipe, VPHeaderPHIRecipe, VPPredInstPHIRecipe>(UI)) {
+          const VPBlockBase *Incoming = nullptr;
+          // Get the incoming block based on the number of predecessors.
+          if (UserVPBB->getNumPredecessors() == 0) {
+            assert((isa<VPWidenPHIRecipe>(UI) || isa<VPHeaderPHIRecipe>(UI)) &&
+                   "Unexpected recipe with 0 predecessors");
+            const VPRegionBlock *UserRegion = UserVPBB->getParent();
+            Incoming = V == UI->getOperand(0)
+                           ? UserRegion->getSinglePredecessor()
+                           : UserRegion->getExiting();
+          } else if (UserVPBB->getNumPredecessors() == 1) {
+            assert(isa<VPWidenPHIRecipe>(UI) &&
+                   "Unexpected recipe with 1 predecessors");
+            Incoming = UserVPBB->getSinglePredecessor();
+          } else {
+            assert(UserVPBB->getNumPredecessors() == 2 &&
+                   isa<VPPredInstPHIRecipe>(UI) &&
+                   "Unexpected recipe with 2 or more predecessors");
+            Incoming = UserVPBB->getPredecessors()[1];
+          }
+          if (auto *R = dyn_cast<VPRegionBlock>(Incoming))
+            Incoming = R->getExiting();
+          if (Incoming != VPBB && !VPDT.dominates(VPBB, Incoming)) {
+            errs() << "Use before def!\n";
+            return false;
+          }
+          continue;
+        }
 
         // If the user is in the same block, check it comes after R in the
         // block.
-        if (UI->getParent() == VPBB) {
+        if (UserVPBB == VPBB) {
           if (RecipeNumbering[UI] < RecipeNumbering[&R]) {
             errs() << "Use before def!\n";
             return false;
@@ -222,7 +270,7 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
           continue;
         }
 
-        if (!VPDT.dominates(VPBB, UI->getParent())) {
+        if (!VPDT.dominates(VPBB, UserVPBB)) {
           errs() << "Use before def!\n";
           return false;
         }

>From 695ef3c6bc9e784dbae48bb35cef50d55bfa666c Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 29 Jan 2025 20:46:13 +0000
Subject: [PATCH 2/2] !fixup add assert for case where Incoming is a region

---
 llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index 0948452e8d19cf..d292c6975cf32b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -180,6 +180,7 @@ static bool isVPIRInstructionPhi(const VPRecipeBase &R) {
   auto *VPIRI = dyn_cast<VPIRInstruction>(&R);
   return VPIRI && isa<PHINode>(VPIRI->getInstruction());
 }
+
 bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
   if (!verifyPhiRecipes(VPBB))
     return false;
@@ -251,8 +252,12 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
                    "Unexpected recipe with 2 or more predecessors");
             Incoming = UserVPBB->getPredecessors()[1];
           }
-          if (auto *R = dyn_cast<VPRegionBlock>(Incoming))
+          if (auto *R = dyn_cast<VPRegionBlock>(Incoming)) {
+            assert(isa<VPWidenPHIRecipe>(UI) &&
+                   "Should only have a region as predecessor for "
+                   "VPWidenPHIRecipe");
             Incoming = R->getExiting();
+          }
           if (Incoming != VPBB && !VPDT.dominates(VPBB, Incoming)) {
             errs() << "Use before def!\n";
             return false;