[llvm] [VPlan] Fix header phi VPInstruction verification. NFC (PR #151472)

[Luke Lau via llvm-commits]

https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/151472

>From 1ac30715bd635141afffa411820bb64280d9e523 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Thu, 31 Jul 2025 16:24:49 +0800
Subject: [PATCH 1/4] [VPlan] Fix header phi VPInstruction verification. NFC

Noticed this when checking the invariant that all phis in the header block must be header phis. I think there's a missing set of parentheses here, since otherwise it only cast<VPInstruction> when RecipeI isn't a VPInstruction.
---
 llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index 57d01cbefbe26..b0de97ac5618d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -80,8 +80,8 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
       NumActiveLaneMaskPhiRecipes++;
 
     if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe>(*RecipeI) &&
-        !isa<VPInstruction>(*RecipeI) &&
-        cast<VPInstruction>(RecipeI)->getOpcode() == Instruction::PHI) {
+        !(isa<VPInstruction>(*RecipeI) &&
+          cast<VPInstruction>(RecipeI)->getOpcode() == Instruction::PHI)) {
       errs() << "Found non-header PHI recipe in header VPBB";
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
       errs() << ": ";

>From c170996dca9e193166cba01c539c426a30d6c10f Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Thu, 31 Jul 2025 17:03:22 +0800
Subject: [PATCH 2/4] Add unit test

---
 .../Vectorize/VPlanVerifierTest.cpp           | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index 6214ea3a7dc74..47d4239b30595 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -287,6 +287,38 @@ TEST_F(VPVerifierTest, BlockOutsideRegionWithParent) {
 #endif
 }
 
+TEST_F(VPVerifierTest, NonHeaderPHIInHeader) {
+  VPlan &Plan = getPlan();
+  VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+  auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
+  VPInstruction *BranchOnCond =
+      new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
+
+  VPBasicBlock *VPBB1 = Plan.getEntry();
+  VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+
+  VPBB2->appendRecipe(CanIV);
+
+  PHINode *PHINode = PHINode::Create(Type::getInt32Ty(C), 2);
+  VPIRPhi *IRPhi = new VPIRPhi(*PHINode);
+  VPBB2->appendRecipe(IRPhi);
+  VPBB2->appendRecipe(BranchOnCond);
+
+  VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB2, VPBB2, "R1");
+  VPBlockUtils::connectBlocks(VPBB1, R1);
+  VPBlockUtils::connectBlocks(R1, Plan.getScalarHeader());
+
+#if GTEST_HAS_STREAM_REDIRECTION
+  ::testing::internal::CaptureStderr();
+#endif
+  EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+  EXPECT_STREQ(
+      "Found non-header PHI recipe in header VPBB: IR   <badref> = phi i32 \n",
+      ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
 class VPIRVerifierTest : public VPlanTestIRBase {};
 
 TEST_F(VPIRVerifierTest, testVerifyIRPhi) {

>From f11ad6137b65ece31074b38f04d067dd2a1c91f8 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Thu, 31 Jul 2025 17:05:52 +0800
Subject: [PATCH 3/4] Use isa<VPPhi>

---
 llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index b0de97ac5618d..5fbff69d60749 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -79,9 +79,8 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
     if (isa<VPActiveLaneMaskPHIRecipe>(RecipeI))
       NumActiveLaneMaskPhiRecipes++;
 
-    if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe>(*RecipeI) &&
-        !(isa<VPInstruction>(*RecipeI) &&
-          cast<VPInstruction>(RecipeI)->getOpcode() == Instruction::PHI)) {
+    if (IsHeaderVPBB &&
+        !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe, VPPhi>(*RecipeI)) {
       errs() << "Found non-header PHI recipe in header VPBB";
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
       errs() << ": ";

>From 77f1e7f8685b4b91b182079e7562ef014da15356 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Thu, 31 Jul 2025 17:39:21 +0800
Subject: [PATCH 4/4] Address comments

---
 llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index 47d4239b30595..20912200bf653 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -291,16 +291,15 @@ TEST_F(VPVerifierTest, NonHeaderPHIInHeader) {
   VPlan &Plan = getPlan();
   VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
   auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
-  VPInstruction *BranchOnCond =
-      new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
+  auto *BranchOnCond = new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
 
   VPBasicBlock *VPBB1 = Plan.getEntry();
-  VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+  VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("header");
 
   VPBB2->appendRecipe(CanIV);
 
   PHINode *PHINode = PHINode::Create(Type::getInt32Ty(C), 2);
-  VPIRPhi *IRPhi = new VPIRPhi(*PHINode);
+  auto *IRPhi = new VPIRPhi(*PHINode);
   VPBB2->appendRecipe(IRPhi);
   VPBB2->appendRecipe(BranchOnCond);