[llvm] [VPlan] Verify scalar types in VPlanVerifier. NFCI (PR #122679)
Luke Lau via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 20:41:49 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/4] [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/4] 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});
>From d720360bc34327bbe1285bcb991631c91b4f608c Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 14 Jan 2025 12:34:41 +0800
Subject: [PATCH 3/4] Change back LogicalAnd and add assert for its operand
---
llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index bc84757420e834..1579a81ae5e2fa 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -77,7 +77,6 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
case VPInstruction::CalculateTripCountMinusVF:
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::AnyOf:
- case VPInstruction::LogicalAnd:
return SetResultTyFromOp();
case VPInstruction::ExtractFromEnd: {
Type *BaseTy = inferScalarType(R->getOperand(0));
@@ -85,6 +84,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
return VecTy->getElementType();
return BaseTy;
}
+ case VPInstruction::LogicalAnd:
+ assert(inferScalarType(R->getOperand(0))->isIntegerTy(1) &&
+ "LogicalAnd operand should be bool");
+ 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));
>From 1b6111ab7aa81119a4ea290919146c451dc4117f Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 14 Jan 2025 12:40:12 +0800
Subject: [PATCH 4/4] Update comment on AnyOf
---
llvm/lib/Transforms/Vectorize/VPlan.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index cfbb4ad32d6810..83bee631c2fc86 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1222,8 +1222,8 @@ class VPInstruction : public VPRecipeWithIRFlags,
// operand). Only generates scalar values (either for the first lane only or
// for all lanes, depending on its uses).
PtrAdd,
- // Returns a scalar boolean value, which is true if any lane of its single
- // operand is true.
+ // Returns a scalar boolean value, which is true if any lane of its (only
+ // boolean) vector operand is true.
AnyOf,
};
More information about the llvm-commits
mailing list