Skip to content

Commit 5c15caa

Browse files
authored
[VPlan] Verify scalar types in VPlanVerifier. NFCI (#122679)
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 ICmp and ActiveLaneMask to return i1 (to align with `icmp` and `@llvm.get.active.lane.mask` in the LangRef) The VPlanVerifier unit tests also need to be fleshed out a bit more to satisfy the stricter assertions
1 parent 13b4428 commit 5c15caa

File tree

4 files changed

+51
-20
lines changed

4 files changed

+51
-20
lines changed

llvm/lib/Transforms/Vectorize/VPlan.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,8 +1223,8 @@ class VPInstruction : public VPRecipeWithIRFlags,
12231223
// operand). Only generates scalar values (either for the first lane only or
12241224
// for all lanes, depending on its uses).
12251225
PtrAdd,
1226-
// Returns a scalar boolean value, which is true if any lane of its single
1227-
// operand is true.
1226+
// Returns a scalar boolean value, which is true if any lane of its (only
1227+
// boolean) vector operand is true.
12281228
AnyOf,
12291229
};
12301230

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
6060
}
6161
case Instruction::ICmp:
6262
case VPInstruction::ActiveLaneMask:
63-
return inferScalarType(R->getOperand(1));
63+
assert(inferScalarType(R->getOperand(0)) ==
64+
inferScalarType(R->getOperand(1)) &&
65+
"different types inferred for different operands");
66+
return IntegerType::get(Ctx, 1);
6467
case VPInstruction::ComputeReductionResult: {
6568
auto *PhiR = cast<VPReductionPHIRecipe>(R->getOperand(0));
6669
auto *OrigPhi = cast<PHINode>(PhiR->getUnderlyingValue());
@@ -71,6 +74,9 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
7174
case VPInstruction::FirstOrderRecurrenceSplice:
7275
case VPInstruction::Not:
7376
case VPInstruction::ResumePhi:
77+
case VPInstruction::CalculateTripCountMinusVF:
78+
case VPInstruction::CanonicalIVIncrementForPart:
79+
case VPInstruction::AnyOf:
7480
return SetResultTyFromOp();
7581
case VPInstruction::ExtractFromEnd: {
7682
Type *BaseTy = inferScalarType(R->getOperand(0));
@@ -79,6 +85,9 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
7985
return BaseTy;
8086
}
8187
case VPInstruction::LogicalAnd:
88+
assert(inferScalarType(R->getOperand(0))->isIntegerTy(1) &&
89+
inferScalarType(R->getOperand(1))->isIntegerTy(1) &&
90+
"LogicalAnd operands should be bool");
8291
return IntegerType::get(Ctx, 1);
8392
case VPInstruction::PtrAdd:
8493
// Return the type based on the pointer argument (i.e. first operand).

llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ using namespace llvm;
2626
namespace {
2727
class VPlanVerifier {
2828
const VPDominatorTree &VPDT;
29+
VPTypeAnalysis &TypeInfo;
2930

3031
SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;
3132

@@ -58,7 +59,8 @@ class VPlanVerifier {
5859
bool verifyRegionRec(const VPRegionBlock *Region);
5960

6061
public:
61-
VPlanVerifier(VPDominatorTree &VPDT) : VPDT(VPDT) {}
62+
VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo)
63+
: VPDT(VPDT), TypeInfo(TypeInfo) {}
6264

6365
bool verify(const VPlan &Plan);
6466
};
@@ -195,6 +197,14 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
195197
return false;
196198
}
197199
for (const VPValue *V : R.definedValues()) {
200+
// Verify that we can infer a scalar type for each defined value. With
201+
// assertions enabled, inferScalarType will perform some consistency
202+
// checks during type inference.
203+
if (!TypeInfo.inferScalarType(V)) {
204+
errs() << "Failed to infer scalar type!\n";
205+
return false;
206+
}
207+
198208
for (const VPUser *U : V->users()) {
199209
auto *UI = dyn_cast<VPRecipeBase>(U);
200210
// TODO: check dominance of incoming values for phis properly.
@@ -406,6 +416,8 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
406416
bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
407417
VPDominatorTree VPDT;
408418
VPDT.recalculate(const_cast<VPlan &>(Plan));
409-
VPlanVerifier Verifier(VPDT);
419+
VPTypeAnalysis TypeInfo(
420+
const_cast<VPlan &>(Plan).getCanonicalIV()->getScalarType());
421+
VPlanVerifier Verifier(VPDT, TypeInfo);
410422
return Verifier.verify(Plan);
411423
}

llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@ using VPVerifierTest = VPlanTestBase;
2020
namespace {
2121
TEST_F(VPVerifierTest, VPInstructionUseBeforeDefSameBB) {
2222
VPlan &Plan = getPlan();
23-
VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
23+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
24+
VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
2425
VPInstruction *UseI = new VPInstruction(Instruction::Sub, {DefI});
26+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
2527

2628
VPBasicBlock *VPBB1 = Plan.getEntry();
2729
VPBB1->appendRecipe(UseI);
2830
VPBB1->appendRecipe(DefI);
2931

3032
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
33+
VPBB2->appendRecipe(CanIV);
3134
VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB2, VPBB2, "R1");
3235
VPBlockUtils::connectBlocks(VPBB1, R1);
3336
VPBlockUtils::connectBlocks(R1, Plan.getScalarHeader());
@@ -44,9 +47,10 @@ TEST_F(VPVerifierTest, VPInstructionUseBeforeDefSameBB) {
4447

4548
TEST_F(VPVerifierTest, VPInstructionUseBeforeDefDifferentBB) {
4649
VPlan &Plan = getPlan();
47-
VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
50+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
51+
VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
4852
VPInstruction *UseI = new VPInstruction(Instruction::Sub, {DefI});
49-
auto *CanIV = new VPCanonicalIVPHIRecipe(UseI, {});
53+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
5054
VPInstruction *BranchOnCond =
5155
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
5256

@@ -73,23 +77,22 @@ TEST_F(VPVerifierTest, VPInstructionUseBeforeDefDifferentBB) {
7377
}
7478

7579
TEST_F(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
80+
VPlan &Plan = getPlan();
7681
IntegerType *Int32 = IntegerType::get(C, 32);
7782
auto *Phi = PHINode::Create(Int32, 1);
83+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 0));
7884

79-
VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
80-
VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
81-
auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
85+
VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
86+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
8287
VPInstruction *BranchOnCond =
8388
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
8489
auto *Blend = new VPBlendRecipe(Phi, {DefI});
8590

86-
VPlan &Plan = getPlan();
8791
VPBasicBlock *VPBB1 = Plan.getEntry();
8892
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
8993
VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
9094
VPBasicBlock *VPBB4 = Plan.createVPBasicBlock("");
9195

92-
VPBB1->appendRecipe(I1);
9396
VPBB2->appendRecipe(CanIV);
9497
VPBB3->appendRecipe(Blend);
9598
VPBB4->appendRecipe(DefI);
@@ -116,14 +119,15 @@ TEST_F(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
116119
}
117120

118121
TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
119-
VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
120-
auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
122+
VPlan &Plan = getPlan();
123+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
124+
VPInstruction *I1 = new VPInstruction(Instruction::Add, {Zero});
125+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
121126
VPInstruction *BranchOnCond =
122127
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
123128
VPInstruction *BranchOnCond2 =
124129
new VPInstruction(VPInstruction::BranchOnCond, {I1});
125130

126-
VPlan &Plan = getPlan();
127131
VPBasicBlock *VPBB1 = Plan.getEntry();
128132
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
129133

@@ -149,14 +153,15 @@ TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
149153
}
150154

151155
TEST_F(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
152-
VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
153-
auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
156+
VPlan &Plan = getPlan();
157+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
158+
VPInstruction *I1 = new VPInstruction(Instruction::Add, {Zero});
159+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
154160
VPInstruction *BranchOnCond =
155161
new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
156162
VPInstruction *BranchOnCond2 =
157163
new VPInstruction(VPInstruction::BranchOnCond, {I1});
158164

159-
VPlan &Plan = getPlan();
160165
VPBasicBlock *VPBB1 = Plan.getEntry();
161166
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
162167
VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
@@ -186,10 +191,15 @@ TEST_F(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
186191

187192
TEST_F(VPVerifierTest, BlockOutsideRegionWithParent) {
188193
VPlan &Plan = getPlan();
194+
189195
VPBasicBlock *VPBB1 = Plan.getEntry();
190196
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
191197

192-
VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
198+
VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));
199+
auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {});
200+
VPBB2->appendRecipe(CanIV);
201+
202+
VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
193203
VPInstruction *BranchOnCond =
194204
new VPInstruction(VPInstruction::BranchOnCond, {DefI});
195205

0 commit comments

Comments
 (0)