[llvm] [VPlan] Verify scalar types in VPlanVerifier. NFCI (PR #122679)
Luke Lau via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 07:46:58 PST 2025
https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/122679
>From 9de113e64cd9fc778e021348f6b1be1be0aef41f Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 13 Jan 2025 17:53:19 +0800
Subject: [PATCH 1/2] [VPlan] Verify scalar types in VPlanVerifier. NFCI
VTypeAnalysis contains some assertions which can be useful for reasoning that the types of various operands match.
This patch teaches VPlanVerifier to invoke VTypeAnalysis to check them, and catches some issues with VPInstruction types that are also fixed here:
* Handles the missing cases for CalculateTripCountMinusVF, CanonicalIVIncrementForPart and AnyOf
* Fixes LogicalAnd to return its operands' type (to align with `and` in the LangRef)
* Fixes ICmp and ActiveLaneMask to return i1 (to align with `icmp` and `@llvm.get.active.lane.mask` in the LangRef)
---
llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp | 11 ++++++++---
llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp | 11 +++++++++--
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index 35497a7431f766..bc84757420e834 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -60,7 +60,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
}
case Instruction::ICmp:
case VPInstruction::ActiveLaneMask:
- return inferScalarType(R->getOperand(1));
+ assert(inferScalarType(R->getOperand(0)) ==
+ inferScalarType(R->getOperand(1)) &&
+ "different types inferred for different operands");
+ return IntegerType::get(Ctx, 1);
case VPInstruction::ComputeReductionResult: {
auto *PhiR = cast<VPReductionPHIRecipe>(R->getOperand(0));
auto *OrigPhi = cast<PHINode>(PhiR->getUnderlyingValue());
@@ -71,6 +74,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
case VPInstruction::FirstOrderRecurrenceSplice:
case VPInstruction::Not:
case VPInstruction::ResumePhi:
+ case VPInstruction::CalculateTripCountMinusVF:
+ case VPInstruction::CanonicalIVIncrementForPart:
+ case VPInstruction::AnyOf:
+ case VPInstruction::LogicalAnd:
return SetResultTyFromOp();
case VPInstruction::ExtractFromEnd: {
Type *BaseTy = inferScalarType(R->getOperand(0));
@@ -78,8 +85,6 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
return VecTy->getElementType();
return BaseTy;
}
- case VPInstruction::LogicalAnd:
- return IntegerType::get(Ctx, 1);
case VPInstruction::PtrAdd:
// Return the type based on the pointer argument (i.e. first operand).
return inferScalarType(R->getOperand(0));
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index be420a873bef52..647c03ec69ba4f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -26,6 +26,7 @@ using namespace llvm;
namespace {
class VPlanVerifier {
const VPDominatorTree &VPDT;
+ VPTypeAnalysis &TypeInfo;
SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;
@@ -58,7 +59,8 @@ class VPlanVerifier {
bool verifyRegionRec(const VPRegionBlock *Region);
public:
- VPlanVerifier(VPDominatorTree &VPDT) : VPDT(VPDT) {}
+ VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo)
+ : VPDT(VPDT), TypeInfo(TypeInfo) {}
bool verify(const VPlan &Plan);
};
@@ -195,6 +197,9 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
return false;
}
for (const VPValue *V : R.definedValues()) {
+ // Verify that recipes' operands have matching types.
+ TypeInfo.inferScalarType(V);
+
for (const VPUser *U : V->users()) {
auto *UI = dyn_cast<VPRecipeBase>(U);
// TODO: check dominance of incoming values for phis properly.
@@ -406,6 +411,8 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
VPDominatorTree VPDT;
VPDT.recalculate(const_cast<VPlan &>(Plan));
- VPlanVerifier Verifier(VPDT);
+ VPTypeAnalysis TypeInfo(
+ const_cast<VPlan &>(Plan).getCanonicalIV()->getScalarType());
+ VPlanVerifier Verifier(VPDT, TypeInfo);
return Verifier.verify(Plan);
}
>From 50727a005633dc37eca2690b8e51fadb66ea31b0 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 13 Jan 2025 23:46:17 +0800
Subject: [PATCH 2/2] Fix verifier unit tests by filling out stuff necessary
for corrrect types
---
.../Vectorize/VPlanVerifierTest.cpp | 40 ++++++++++++-------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index f098ba0bce497b..f818b49fdbe7fe 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -20,14 +20,17 @@ using VPVerifierTest = VPlanTestBase;
namespace {
TEST_F(VPVerifierTest, VPInstructionUseBeforeDefSameBB) {
VPlan &Plan = getPlan();
- VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+ VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
VPInstruction *UseI = new VPInstruction(Instruction::Sub, {DefI});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBB1->appendRecipe(UseI);
VPBB1->appendRecipe(DefI);
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+ VPBB2->appendRecipe(CanIV);
VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB2, VPBB2, "R1");
VPBlockUtils::connectBlocks(VPBB1, R1);
VPBlockUtils::connectBlocks(R1, Plan.getScalarHeader());
@@ -44,9 +47,10 @@ TEST_F(VPVerifierTest, VPInstructionUseBeforeDefSameBB) {
TEST_F(VPVerifierTest, VPInstructionUseBeforeDefDifferentBB) {
VPlan &Plan = getPlan();
- VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+ VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
VPInstruction *UseI = new VPInstruction(Instruction::Sub, {DefI});
- auto *CanIV = new VPCanonicalIVPHIRecipe(UseI, {});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
@@ -73,23 +77,22 @@ TEST_F(VPVerifierTest, VPInstructionUseBeforeDefDifferentBB) {
}
TEST_F(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
+ VPlan &Plan = getPlan();
IntegerType *Int32 = IntegerType::get(C, 32);
auto *Phi = PHINode::Create(Int32, 1);
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 0));
- VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
- VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
- auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
+ VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
auto *Blend = new VPBlendRecipe(Phi, {DefI});
- VPlan &Plan = getPlan();
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
VPBasicBlock *VPBB4 = Plan.createVPBasicBlock("");
- VPBB1->appendRecipe(I1);
VPBB2->appendRecipe(CanIV);
VPBB3->appendRecipe(Blend);
VPBB4->appendRecipe(DefI);
@@ -116,14 +119,15 @@ TEST_F(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
}
TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
- VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
- auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
+ VPlan &Plan = getPlan();
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+ VPInstruction *I1 = new VPInstruction(Instruction::Add, {Zero});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
VPInstruction *BranchOnCond2 =
new VPInstruction(VPInstruction::BranchOnCond, {I1});
- VPlan &Plan = getPlan();
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
@@ -149,14 +153,15 @@ TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
}
TEST_F(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
- VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
- auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
+ VPlan &Plan = getPlan();
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+ VPInstruction *I1 = new VPInstruction(Instruction::Add, {Zero});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
VPInstruction *BranchOnCond2 =
new VPInstruction(VPInstruction::BranchOnCond, {I1});
- VPlan &Plan = getPlan();
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
@@ -186,10 +191,15 @@ TEST_F(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
TEST_F(VPVerifierTest, BlockOutsideRegionWithParent) {
VPlan &Plan = getPlan();
+
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
- VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
+ VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
+ auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
+ VPBB2->appendRecipe(CanIV);
+
+ VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {DefI});
More information about the llvm-commits
mailing list