From 1ac30715bd635141afffa411820bb64280d9e523 Mon Sep 17 00:00:00 2001 From: Luke Lau 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 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(*RecipeI) && - !isa(*RecipeI) && - cast(RecipeI)->getOpcode() == Instruction::PHI) { + !(isa(*RecipeI) && + cast(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 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 = 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 Date: Thu, 31 Jul 2025 17:05:52 +0800 Subject: [PATCH 3/4] Use isa --- 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(RecipeI)) NumActiveLaneMaskPhiRecipes++; - if (IsHeaderVPBB && !isa(*RecipeI) && - !(isa(*RecipeI) && - cast(RecipeI)->getOpcode() == Instruction::PHI)) { + if (IsHeaderVPBB && + !isa(*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 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);