Skip to content

Commit 8c97ee5

Browse files
fhahngithub-actions[bot]
authored andcommitted
Automerge: [VPlan] Verify incoming values of VPIRPhi matches before checking (NFC)
Update the verifier to first check if the number of incoming values matches the number of predecessors, before using incoming_values_and_blocks. We unfortunately need also check here, as this may be called before verifyPhiRecipes runs. Also update the verifier unit tests, to actually fail for the expected recipes.
2 parents 73a18fd + 290ff95 commit 8c97ee5

File tree

2 files changed

+51
-7
lines changed

2 files changed

+51
-7
lines changed

llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,13 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
252252

253253
for (const VPUser *U : V->users()) {
254254
auto *UI = cast<VPRecipeBase>(U);
255+
if (isa<VPIRPhi>(UI) &&
256+
UI->getNumOperands() != UI->getParent()->getNumPredecessors()) {
257+
errs() << "Phi-like recipe with different number of operands and "
258+
"predecessors.\n";
259+
return false;
260+
}
261+
255262
if (auto *Phi = dyn_cast<VPPhiAccessors>(UI)) {
256263
for (const auto &[IncomingVPV, IncomingVPBB] :
257264
Phi->incoming_values_and_blocks()) {

llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -326,22 +326,18 @@ TEST_F(VPVerifierTest, NonHeaderPHIInHeader) {
326326

327327
class VPIRVerifierTest : public VPlanTestIRBase {};
328328

329-
TEST_F(VPIRVerifierTest, testVerifyIRPhi) {
329+
TEST_F(VPIRVerifierTest, testVerifyIRPhiInScalarHeaderVPIRBB) {
330330
const char *ModuleString =
331331
"define void @f(ptr %A, i64 %N) {\n"
332332
"entry:\n"
333333
" br label %loop\n"
334334
"loop:\n"
335335
" %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]\n"
336-
" %arr.idx = getelementptr inbounds i32, ptr %A, i64 %iv\n"
337-
" %l1 = load i32, ptr %arr.idx, align 4\n"
338-
" %res = add i32 %l1, 10\n"
339-
" store i32 %res, ptr %arr.idx, align 4\n"
340336
" %iv.next = add i64 %iv, 1\n"
341337
" %exitcond = icmp ne i64 %iv.next, %N\n"
342338
" br i1 %exitcond, label %loop, label %for.end\n"
343339
"for.end:\n"
344-
" %p = phi i32 [ %l1, %loop ]\n"
340+
" %p = phi i64 [ %iv, %loop ]\n"
345341
" ret void\n"
346342
"}\n";
347343

@@ -351,7 +347,48 @@ TEST_F(VPIRVerifierTest, testVerifyIRPhi) {
351347
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
352348
auto Plan = buildVPlan(LoopHeader);
353349

354-
Plan->getExitBlocks()[0]->front().addOperand(Plan->getConstantInt(32, 0));
350+
#if GTEST_HAS_STREAM_REDIRECTION
351+
::testing::internal::CaptureStderr();
352+
#endif
353+
EXPECT_FALSE(verifyVPlanIsValid(*Plan));
354+
#if GTEST_HAS_STREAM_REDIRECTION
355+
EXPECT_STREQ(
356+
"Phi-like recipe with different number of operands and predecessors.\n",
357+
::testing::internal::GetCapturedStderr().c_str());
358+
#endif
359+
}
360+
361+
TEST_F(VPIRVerifierTest, testVerifyIRPhiInExitVPIRBB) {
362+
const char *ModuleString =
363+
"define void @f(ptr %A, i64 %N) {\n"
364+
"entry:\n"
365+
" br label %loop\n"
366+
"loop:\n"
367+
" %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]\n"
368+
" %iv.next = add i64 %iv, 1\n"
369+
" %exitcond = icmp ne i64 %iv.next, %N\n"
370+
" br i1 %exitcond, label %loop, label %for.end\n"
371+
"for.end:\n"
372+
" %p = phi i64 [ %iv, %loop ]\n"
373+
" ret void\n"
374+
"}\n";
375+
376+
Module &M = parseModule(ModuleString);
377+
378+
Function *F = M.getFunction("f");
379+
BasicBlock *LoopHeader = F->getEntryBlock().getSingleSuccessor();
380+
auto Plan = buildVPlan(LoopHeader);
381+
382+
// Create a definition in the vector loop header that will be used by the phi.
383+
auto *HeaderBlock =
384+
cast<VPBasicBlock>(Plan->getVectorLoopRegion()->getEntry());
385+
VPInstruction *DefI =
386+
new VPInstruction(VPInstruction::ExtractLastElement,
387+
{HeaderBlock->front().getVPSingleValue()});
388+
DefI->insertBefore(Plan->getMiddleBlock()->getTerminator());
389+
Plan->getExitBlocks()[0]->front().addOperand(DefI);
390+
VPValue *Zero = Plan->getConstantInt(32, 0);
391+
Plan->getScalarHeader()->front().addOperand(Zero);
355392

356393
#if GTEST_HAS_STREAM_REDIRECTION
357394
::testing::internal::CaptureStderr();

0 commit comments

Comments
 (0)