diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp --- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp @@ -15,6 +15,7 @@ #include "VPlanVerifier.h" #include "VPlan.h" #include "VPlanCFG.h" +#include "VPlanDominatorTree.h" #include "llvm/ADT/DepthFirstIterator.h" #include "llvm/Support/CommandLine.h" @@ -189,9 +190,8 @@ return true; } -static bool -verifyVPBasicBlock(const VPBasicBlock *VPBB, - DenseMap &BlockNumbering) { +static bool verifyVPBasicBlock(const VPBasicBlock *VPBB, + VPDominatorTree &VPDT) { if (!verifyPhiRecipes(VPBB)) return false; @@ -206,7 +206,8 @@ for (const VPValue *V : R.definedValues()) { for (const VPUser *U : V->users()) { auto *UI = dyn_cast(U); - if (!UI || isa(UI)) + // TODO: check dominance of incoming values for phis properly. + if (!UI || isa(UI) || isa(UI)) continue; // If the user is in the same block, check it comes after R in the @@ -219,27 +220,7 @@ continue; } - // Skip blocks outside any region for now and blocks outside - // replicate-regions. - auto *ParentR = VPBB->getParent(); - if (!ParentR || !ParentR->isReplicator()) - continue; - - // For replicators, verify that VPPRedInstPHIRecipe defs are only used - // in subsequent blocks. - if (isa(&R)) { - auto I = BlockNumbering.find(UI->getParent()); - unsigned BlockNumber = I == BlockNumbering.end() ? std::numeric_limits::max() : I->second; - if (BlockNumber < BlockNumbering[ParentR]) { - errs() << "Use before def!\n"; - return false; - } - continue; - } - - // All non-VPPredInstPHIRecipe recipes in the block must be used in - // the replicate region only. - if (UI->getParent()->getParent() != ParentR) { + if (!VPDT.dominates(VPBB, UI->getParent())) { errs() << "Use before def!\n"; return false; } @@ -250,15 +231,13 @@ } bool VPlanVerifier::verifyPlanIsValid(const VPlan &Plan) { - DenseMap BlockNumbering; - unsigned Cnt = 0; + VPDominatorTree VPDT; + VPDT.recalculate(const_cast(Plan)); + auto Iter = vp_depth_first_deep(Plan.getEntry()); - for (const VPBlockBase *VPB : Iter) { - BlockNumbering[VPB] = Cnt++; - auto *VPBB = dyn_cast(VPB); - if (!VPBB) - continue; - if (!verifyVPBasicBlock(VPBB, BlockNumbering)) + for (const VPBasicBlock *VPBB : + VPBlockUtils::blocksOnly(Iter)) { + if (!verifyVPBasicBlock(VPBB, VPDT)) return false; } diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp --- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp @@ -57,14 +57,13 @@ VPlan Plan; Plan.setEntry(VPBB1); - // TODO: UseI uses DefI but DefI does not dominate UseI. Currently missed by - // the verifier. #if GTEST_HAS_STREAM_REDIRECTION ::testing::internal::CaptureStderr(); #endif - EXPECT_TRUE(VPlanVerifier::verifyPlanIsValid(Plan)); + EXPECT_FALSE(VPlanVerifier::verifyPlanIsValid(Plan)); #if GTEST_HAS_STREAM_REDIRECTION - EXPECT_STREQ("", ::testing::internal::GetCapturedStderr().c_str()); + EXPECT_STREQ("Use before def!\n", + ::testing::internal::GetCapturedStderr().c_str()); #endif } @@ -99,14 +98,13 @@ VPlan Plan; Plan.setEntry(VPBB1); - // TODO: Blend uses Def but Def does not dominate Blend. Currently missed by - // the verifier. #if GTEST_HAS_STREAM_REDIRECTION ::testing::internal::CaptureStderr(); #endif - EXPECT_TRUE(VPlanVerifier::verifyPlanIsValid(Plan)); + EXPECT_FALSE(VPlanVerifier::verifyPlanIsValid(Plan)); #if GTEST_HAS_STREAM_REDIRECTION - EXPECT_STREQ("", ::testing::internal::GetCapturedStderr().c_str()); + EXPECT_STREQ("Use before def!\n", + ::testing::internal::GetCapturedStderr().c_str()); #endif delete Phi;