[llvm] 9923d29 - [VPlan] Merge main VPlan verifer with HCFG verifier.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 20 08:44:16 PST 2024
Author: Florian Hahn
Date: 2024-02-20T16:43:57Z
New Revision: 9923d29cfa917a0c25f3237e0cae9567c8806071
URL: https://github.com/llvm/llvm-project/commit/9923d29cfa917a0c25f3237e0cae9567c8806071
DIFF: https://github.com/llvm/llvm-project/commit/9923d29cfa917a0c25f3237e0cae9567c8806071.diff
LOG: [VPlan] Merge main VPlan verifer with HCFG verifier.
Unify VPlan verifiers in verifyVPlanIsValid. This adds verification for
various properties on blocks to the verifier used for VPlans generated
by the inner loop vectorizer. It also adds def-use checks for the
verifier used in the VPlan native path.
This drops the separate flag to enable HCFG verification. Instead, all
VPlans are verified once they have been created, if assertions are
enabled.
This also removes VPWidenPHIRecipe from VPHeaderPHIRecipe; it is used to
model any phi node in the native path.
Added:
Modified:
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/VPlan.cpp
llvm/lib/Transforms/Vectorize/VPlan.h
llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
llvm/lib/Transforms/Vectorize/VPlanValue.h
llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
llvm/lib/Transforms/Vectorize/VPlanVerifier.h
llvm/test/Transforms/LoopVectorize/vplan_hcfg_stress_test.ll
llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index eca901fcdae4ce..373ea751d568ae 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -60,6 +60,7 @@
#include "VPlanAnalysis.h"
#include "VPlanHCFGBuilder.h"
#include "VPlanTransforms.h"
+#include "VPlanVerifier.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
@@ -8490,7 +8491,7 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
VPlanTransforms::truncateToMinimalBitwidths(
*Plan, CM.getMinimalBitwidths(), PSE.getSE()->getContext());
VPlanTransforms::optimize(*Plan, *PSE.getSE());
- assert(VPlanVerifier::verifyPlanIsValid(*Plan) && "VPlan is invalid");
+ assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid");
VPlans.push_back(std::move(Plan));
}
VF = SubRange.End;
@@ -8825,6 +8826,7 @@ VPlanPtr LoopVectorizationPlanner::buildVPlan(VFRange &Range) {
bool HasNUW = true;
addCanonicalIVRecipes(*Plan, Legal->getWidestInductionType(), HasNUW,
DebugLoc());
+ assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid");
return Plan;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 2c0daa82afa59f..e55db2df82b47b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -559,7 +559,8 @@ static bool hasConditionalTerminator(const VPBasicBlock *VPBB) {
VPI->getOpcode() == VPInstruction::BranchOnCount));
(void)IsCondBranch;
- if (VPBB->getNumSuccessors() >= 2 || VPBB->isExiting()) {
+ if (VPBB->getNumSuccessors() >= 2 ||
+ (VPBB->isExiting() && !VPBB->getParent()->isReplicator())) {
assert(IsCondBranch && "block with multiple successors not terminated by "
"conditional branch recipe");
@@ -585,7 +586,7 @@ const VPRecipeBase *VPBasicBlock::getTerminator() const {
}
bool VPBasicBlock::isExiting() const {
- return getParent()->getExitingBasicBlock() == this;
+ return getParent() && getParent()->getExitingBasicBlock() == this;
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -1333,7 +1334,7 @@ void VPInterleavedAccessInfo::visitBlock(VPBlockBase *Block, Old2NewTy &Old2New,
InterleavedAccessInfo &IAI) {
if (VPBasicBlock *VPBB = dyn_cast<VPBasicBlock>(Block)) {
for (VPRecipeBase &VPI : *VPBB) {
- if (isa<VPHeaderPHIRecipe>(&VPI))
+ if (isa<VPWidenPHIRecipe>(&VPI))
continue;
assert(isa<VPInstruction>(&VPI) && "Can only handle VPInstructions");
auto *VPInst = cast<VPInstruction>(&VPI);
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 4f25059349fd05..a3ecdb99e9d9fd 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1736,17 +1736,17 @@ class VPWidenPointerInductionRecipe : public VPHeaderPHIRecipe {
#endif
};
-/// A recipe for handling header phis that are widened in the vector loop.
+/// A recipe for handling phis that are widened in the vector loop.
/// In the VPlan native path, all incoming VPValues & VPBasicBlock pairs are
/// managed in the recipe directly.
-class VPWidenPHIRecipe : public VPHeaderPHIRecipe {
+class VPWidenPHIRecipe : public VPSingleDefRecipe {
/// List of incoming blocks. Only used in the VPlan native path.
SmallVector<VPBasicBlock *, 2> IncomingBlocks;
public:
/// Create a new VPWidenPHIRecipe for \p Phi with start value \p Start.
VPWidenPHIRecipe(PHINode *Phi, VPValue *Start = nullptr)
- : VPHeaderPHIRecipe(VPDef::VPWidenPHISC, Phi) {
+ : VPSingleDefRecipe(VPDef::VPWidenPHISC, ArrayRef<VPValue *>(), Phi) {
if (Start)
addOperand(Start);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 94456bf858d9c5..6474a9697dce89 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -437,9 +437,6 @@ void VPlanHCFGBuilder::buildHierarchicalCFG() {
buildPlainCFG();
LLVM_DEBUG(Plan.setName("HCFGBuilder: Plain CFG\n"); dbgs() << Plan);
- VPRegionBlock *TopRegion = Plan.getVectorLoopRegion();
- Verifier.verifyHierarchicalCFG(TopRegion);
-
// Compute plain CFG dom tree for VPLInfo.
VPDomTree.recalculate(Plan);
LLVM_DEBUG(dbgs() << "Dominator Tree after building the plain CFG.\n";
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
index 299ae36155cba0..9e8f9f3f400293 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
@@ -25,7 +25,6 @@
#define LLVM_TRANSFORMS_VECTORIZE_VPLAN_VPLANHCFGBUILDER_H
#include "VPlanDominatorTree.h"
-#include "VPlanVerifier.h"
namespace llvm {
@@ -49,9 +48,6 @@ class VPlanHCFGBuilder {
// The VPlan that will contain the H-CFG we are building.
VPlan &Plan;
- // VPlan verifier utility.
- VPlanVerifier Verifier;
-
// Dominator analysis for VPlan plain CFG to be used in the
// construction of the H-CFG. This analysis is no longer valid once regions
// are introduced.
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index c85f7715feaa2a..b114716b7c13d0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -362,13 +362,13 @@ class VPDef {
VPWidenSelectSC,
// START: Phi-like recipes. Need to be kept together.
VPBlendSC,
+ VPWidenPHISC,
VPPredInstPHISC,
// START: SubclassID for recipes that inherit VPHeaderPHIRecipe.
// VPHeaderPHIRecipe need to be kept together.
VPCanonicalIVPHISC,
VPActiveLaneMaskPHISC,
VPFirstOrderRecurrencePHISC,
- VPWidenPHISC,
VPWidenIntOrFpInductionSC,
VPWidenPointerInductionSC,
VPReductionPHISC,
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index d6b81543dbc9cc..5da752f62cfced 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -23,116 +23,6 @@
using namespace llvm;
-static cl::opt<bool> EnableHCFGVerifier("vplan-verify-hcfg", cl::init(false),
- cl::Hidden,
- cl::desc("Verify VPlan H-CFG."));
-
-#ifndef NDEBUG
-/// Utility function that checks whether \p VPBlockVec has duplicate
-/// VPBlockBases.
-static bool hasDuplicates(const SmallVectorImpl<VPBlockBase *> &VPBlockVec) {
- SmallDenseSet<const VPBlockBase *, 8> VPBlockSet;
- for (const auto *Block : VPBlockVec) {
- if (VPBlockSet.count(Block))
- return true;
- VPBlockSet.insert(Block);
- }
- return false;
-}
-#endif
-
-/// Helper function that verifies the CFG invariants of the VPBlockBases within
-/// \p Region. Checks in this function are generic for VPBlockBases. They are
-/// not specific for VPBasicBlocks or VPRegionBlocks.
-static void verifyBlocksInRegion(const VPRegionBlock *Region) {
- for (const VPBlockBase *VPB : vp_depth_first_shallow(Region->getEntry())) {
- // Check block's parent.
- assert(VPB->getParent() == Region && "VPBlockBase has wrong parent");
-
- auto *VPBB = dyn_cast<VPBasicBlock>(VPB);
- // Check block's condition bit.
- if (VPB->getNumSuccessors() > 1 || (VPBB && VPBB->isExiting()))
- assert(VPBB && VPBB->getTerminator() &&
- "Block has multiple successors but doesn't "
- "have a proper branch recipe!");
- else
- assert((!VPBB || !VPBB->getTerminator()) && "Unexpected branch recipe!");
-
- // Check block's successors.
- const auto &Successors = VPB->getSuccessors();
- // There must be only one instance of a successor in block's successor list.
- // TODO: This won't work for switch statements.
- assert(!hasDuplicates(Successors) &&
- "Multiple instances of the same successor.");
-
- for (const VPBlockBase *Succ : Successors) {
- // There must be a bi-directional link between block and successor.
- const auto &SuccPreds = Succ->getPredecessors();
- assert(llvm::is_contained(SuccPreds, VPB) && "Missing predecessor link.");
- (void)SuccPreds;
- }
-
- // Check block's predecessors.
- const auto &Predecessors = VPB->getPredecessors();
- // There must be only one instance of a predecessor in block's predecessor
- // list.
- // TODO: This won't work for switch statements.
- assert(!hasDuplicates(Predecessors) &&
- "Multiple instances of the same predecessor.");
-
- for (const VPBlockBase *Pred : Predecessors) {
- // Block and predecessor must be inside the same region.
- assert(Pred->getParent() == VPB->getParent() &&
- "Predecessor is not in the same region.");
-
- // There must be a bi-directional link between block and predecessor.
- const auto &PredSuccs = Pred->getSuccessors();
- assert(llvm::is_contained(PredSuccs, VPB) && "Missing successor link.");
- (void)PredSuccs;
- }
- }
-}
-
-/// Verify the CFG invariants of VPRegionBlock \p Region and its nested
-/// VPBlockBases. Do not recurse inside nested VPRegionBlocks.
-static void verifyRegion(const VPRegionBlock *Region) {
- const VPBlockBase *Entry = Region->getEntry();
- const VPBlockBase *Exiting = Region->getExiting();
-
- // Entry and Exiting shouldn't have any predecessor/successor, respectively.
- assert(!Entry->getNumPredecessors() && "Region entry has predecessors.");
- assert(!Exiting->getNumSuccessors() &&
- "Region exiting block has successors.");
- (void)Entry;
- (void)Exiting;
-
- verifyBlocksInRegion(Region);
-}
-
-/// Verify the CFG invariants of VPRegionBlock \p Region and its nested
-/// VPBlockBases. Recurse inside nested VPRegionBlocks.
-static void verifyRegionRec(const VPRegionBlock *Region) {
- verifyRegion(Region);
-
- // Recurse inside nested regions.
- for (const VPBlockBase *VPB : make_range(
- df_iterator<const VPBlockBase *>::begin(Region->getEntry()),
- df_iterator<const VPBlockBase *>::end(Region->getExiting()))) {
- if (const auto *SubRegion = dyn_cast<VPRegionBlock>(VPB))
- verifyRegionRec(SubRegion);
- }
-}
-
-void VPlanVerifier::verifyHierarchicalCFG(
- const VPRegionBlock *TopRegion) const {
- if (!EnableHCFGVerifier)
- return;
-
- LLVM_DEBUG(dbgs() << "Verifying VPlan H-CFG.\n");
- assert(!TopRegion->getParent() && "VPlan Top Region should have no parent.");
- verifyRegionRec(TopRegion);
-}
-
// Verify that phi-like recipes are at the beginning of \p VPBB, with no
// other recipes in between. Also check that only header blocks contain
// VPHeaderPHIRecipes.
@@ -147,7 +37,7 @@ static bool verifyPhiRecipes(const VPBasicBlock *VPBB) {
if (isa<VPActiveLaneMaskPHIRecipe>(RecipeI))
NumActiveLaneMaskPhiRecipes++;
- if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe>(*RecipeI)) {
+ if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe>(*RecipeI)) {
errs() << "Found non-header PHI recipe in header VPBB";
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
errs() << ": ";
@@ -191,7 +81,7 @@ static bool verifyPhiRecipes(const VPBasicBlock *VPBB) {
}
static bool verifyVPBasicBlock(const VPBasicBlock *VPBB,
- VPDominatorTree &VPDT) {
+ const VPDominatorTree &VPDT) {
if (!verifyPhiRecipes(VPBB))
return false;
@@ -207,7 +97,8 @@ static bool verifyVPBasicBlock(const VPBasicBlock *VPBB,
for (const VPUser *U : V->users()) {
auto *UI = dyn_cast<VPRecipeBase>(U);
// TODO: check dominance of incoming values for phis properly.
- if (!UI || isa<VPHeaderPHIRecipe>(UI) || isa<VPPredInstPHIRecipe>(UI))
+ if (!UI ||
+ isa<VPHeaderPHIRecipe, VPWidenPHIRecipe, VPPredInstPHIRecipe>(UI))
continue;
// If the user is in the same block, check it comes after R in the
@@ -230,18 +121,153 @@ static bool verifyVPBasicBlock(const VPBasicBlock *VPBB,
return true;
}
-bool VPlanVerifier::verifyPlanIsValid(const VPlan &Plan) {
- VPDominatorTree VPDT;
- VPDT.recalculate(const_cast<VPlan &>(Plan));
+/// Utility function that checks whether \p VPBlockVec has duplicate
+/// VPBlockBases.
+static bool hasDuplicates(const SmallVectorImpl<VPBlockBase *> &VPBlockVec) {
+ SmallDenseSet<const VPBlockBase *, 8> VPBlockSet;
+ for (const auto *Block : VPBlockVec) {
+ if (VPBlockSet.count(Block))
+ return true;
+ VPBlockSet.insert(Block);
+ }
+ return false;
+}
- auto Iter = vp_depth_first_deep(Plan.getEntry());
- for (const VPBasicBlock *VPBB :
- VPBlockUtils::blocksOnly<const VPBasicBlock>(Iter)) {
- if (!verifyVPBasicBlock(VPBB, VPDT))
+static bool verifyBlock(const VPBlockBase *VPB, const VPDominatorTree &VPDT) {
+ auto *VPBB = dyn_cast<VPBasicBlock>(VPB);
+ if (VPBB && !verifyVPBasicBlock(VPBB, VPDT))
+ return false;
+
+ // Check block's condition bit.
+ if (VPB->getNumSuccessors() > 1 ||
+ (VPBB && VPBB->getParent() && VPBB->isExiting() &&
+ !VPBB->getParent()->isReplicator())) {
+ if (!VPBB || !VPBB->getTerminator()) {
+ errs() << "Block has multiple successors but doesn't "
+ "have a proper branch recipe!\n";
+ return false;
+ }
+ } else {
+ if (VPBB && VPBB->getTerminator()) {
+ errs() << "Unexpected branch recipe!\n";
return false;
+ }
+ }
+
+ // Check block's successors.
+ const auto &Successors = VPB->getSuccessors();
+ // There must be only one instance of a successor in block's successor list.
+ // TODO: This won't work for switch statements.
+ if (hasDuplicates(Successors)) {
+ errs() << "Multiple instances of the same successor.\n";
+ return false;
}
+ for (const VPBlockBase *Succ : Successors) {
+ // There must be a bi-directional link between block and successor.
+ const auto &SuccPreds = Succ->getPredecessors();
+ if (!is_contained(SuccPreds, VPB)) {
+ errs() << "Missing predecessor link.\n";
+ return false;
+ }
+ }
+
+ // Check block's predecessors.
+ const auto &Predecessors = VPB->getPredecessors();
+ // There must be only one instance of a predecessor in block's predecessor
+ // list.
+ // TODO: This won't work for switch statements.
+ if (hasDuplicates(Predecessors)) {
+ errs() << "Multiple instances of the same predecessor.\n";
+ return false;
+ }
+
+ for (const VPBlockBase *Pred : Predecessors) {
+ // Block and predecessor must be inside the same region.
+ if (Pred->getParent() != VPB->getParent()) {
+ errs() << "Predecessor is not in the same region.\n";
+ return false;
+ }
+
+ // There must be a bi-directional link between block and predecessor.
+ const auto &PredSuccs = Pred->getSuccessors();
+ if (!is_contained(PredSuccs, VPB)) {
+ errs() << "Missing successor link.\n";
+ return false;
+ }
+ }
+ return true;
+}
+
+/// Helper function that verifies the CFG invariants of the VPBlockBases within
+/// \p Region. Checks in this function are generic for VPBlockBases. They are
+/// not specific for VPBasicBlocks or VPRegionBlocks.
+static bool verifyBlocksInRegion(const VPRegionBlock *Region,
+ const VPDominatorTree &VPDT) {
+ for (const VPBlockBase *VPB : vp_depth_first_shallow(Region->getEntry())) {
+ // Check block's parent.
+ if (VPB->getParent() != Region) {
+ errs() << "VPBlockBase has wrong parent\n";
+ return false;
+ }
+
+ if (!verifyBlock(VPB, VPDT))
+ return false;
+ }
+ return true;
+}
+
+/// Verify the CFG invariants of VPRegionBlock \p Region and its nested
+/// VPBlockBases. Do not recurse inside nested VPRegionBlocks.
+static bool verifyRegion(const VPRegionBlock *Region,
+ const VPDominatorTree &VPDT) {
+ const VPBlockBase *Entry = Region->getEntry();
+ const VPBlockBase *Exiting = Region->getExiting();
+
+ // Entry and Exiting shouldn't have any predecessor/successor, respectively.
+ if (Entry->getNumPredecessors() != 0) {
+ errs() << "region entry block has predecessors\n";
+ return false;
+ }
+ if (Exiting->getNumSuccessors() != 0) {
+ errs() << "region exiting block has successors\n";
+ return false;
+ }
+
+ return verifyBlocksInRegion(Region, VPDT);
+}
+
+/// Verify the CFG invariants of VPRegionBlock \p Region and its nested
+/// VPBlockBases. Recurse inside nested VPRegionBlocks.
+static bool verifyRegionRec(const VPRegionBlock *Region,
+ const VPDominatorTree &VPDT) {
+ // Recurse inside nested regions and check all blocks inside the region.
+ return verifyRegion(Region, VPDT) &&
+ all_of(vp_depth_first_shallow(Region->getEntry()),
+ [&VPDT](const VPBlockBase *VPB) {
+ const auto *SubRegion = dyn_cast<VPRegionBlock>(VPB);
+ return !SubRegion || verifyRegionRec(SubRegion, VPDT);
+ });
+}
+
+bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
+ VPDominatorTree VPDT;
+ VPDT.recalculate(const_cast<VPlan &>(Plan));
+
+ if (any_of(
+ vp_depth_first_shallow(Plan.getEntry()),
+ [&VPDT](const VPBlockBase *VPB) { return !verifyBlock(VPB, VPDT); }))
+ return false;
+
const VPRegionBlock *TopRegion = Plan.getVectorLoopRegion();
+ if (!verifyRegionRec(TopRegion, VPDT))
+ return false;
+
+ if (TopRegion->getParent()) {
+ errs() << "VPlan Top Region should have no parent.\n";
+ return false;
+ }
+
const VPBasicBlock *Entry = dyn_cast<VPBasicBlock>(TopRegion->getEntry());
if (!Entry) {
errs() << "VPlan entry block is not a VPBasicBlock\n";
@@ -274,19 +300,6 @@ bool VPlanVerifier::verifyPlanIsValid(const VPlan &Plan) {
return false;
}
- for (const VPRegionBlock *Region :
- VPBlockUtils::blocksOnly<const VPRegionBlock>(
- vp_depth_first_deep(Plan.getEntry()))) {
- if (Region->getEntry()->getNumPredecessors() != 0) {
- errs() << "region entry block has predecessors\n";
- return false;
- }
- if (Region->getExiting()->getNumSuccessors() != 0) {
- errs() << "region exiting block has successors\n";
- return false;
- }
- }
-
for (const auto &KV : Plan.getLiveOuts())
if (KV.second->getNumOperands() != 1) {
errs() << "live outs must have a single operand\n";
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
index 839c24e2c9f4d3..3ddc49fda36b89 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
@@ -25,24 +25,16 @@
#define LLVM_TRANSFORMS_VECTORIZE_VPLANVERIFIER_H
namespace llvm {
-class VPRegionBlock;
class VPlan;
-/// Struct with utility functions that can be used to check the consistency and
-/// invariants of a VPlan, including the components of its H-CFG.
-struct VPlanVerifier {
- /// Verify the invariants of the H-CFG starting from \p TopRegion. The
- /// verification process comprises the following steps:
- /// 1. Region/Block verification: Check the Region/Block verification
- /// invariants for every region in the H-CFG.
- void verifyHierarchicalCFG(const VPRegionBlock *TopRegion) const;
+/// Verify invariants for general VPlans. Currently it checks the following:
+/// 1. Region/Block verification: Check the Region/Block verification
+/// invariants for every region in the H-CFG.
+/// 2. all phi-like recipes must be at the beginning of a block, with no other
+/// recipes in between. Note that currently there is still an exception for
+/// VPBlendRecipes.
+bool verifyVPlanIsValid(const VPlan &Plan);
- /// Verify invariants for general VPlans. Currently it checks the following:
- /// 1. all phi-like recipes must be at the beginning of a block, with no other
- /// recipes in between. Note that currently there is still an exception for
- /// VPBlendRecipes.
- static bool verifyPlanIsValid(const VPlan &Plan);
-};
} // namespace llvm
#endif //LLVM_TRANSFORMS_VECTORIZE_VPLANVERIFIER_H
diff --git a/llvm/test/Transforms/LoopVectorize/vplan_hcfg_stress_test.ll b/llvm/test/Transforms/LoopVectorize/vplan_hcfg_stress_test.ll
index 261b924ba51dca..89eaca0cfa8c83 100644
--- a/llvm/test/Transforms/LoopVectorize/vplan_hcfg_stress_test.ll
+++ b/llvm/test/Transforms/LoopVectorize/vplan_hcfg_stress_test.ll
@@ -1,12 +1,10 @@
-; RUN: opt < %s -passes=loop-vectorize -enable-vplan-native-path -vplan-build-stress-test -vplan-verify-hcfg -debug-only=loop-vectorize -disable-output 2>&1 | FileCheck %s -check-prefix=VERIFIER
-; RUN: opt < %s -passes=loop-vectorize -enable-vplan-native-path -vplan-build-stress-test -debug-only=loop-vectorize -disable-output 2>&1 | FileCheck %s -check-prefix=NO-VERIFIER -allow-empty
+; RUN: opt < %s -passes=loop-vectorize -enable-vplan-native-path -vplan-build-stress-test -debug-only=loop-vectorize -disable-output 2>&1 | FileCheck %s
; REQUIRES: asserts
; Verify that the stress testing flag for the VPlan H-CFG builder works as
; expected with and without enabling the VPlan H-CFG Verifier.
-; VERIFIER: Verifying VPlan H-CFG.
-; NO-VERIFIER-NOT: Verifying VPlan H-CFG.
+; CHECK: VPlan 'HCFGBuilder: Plain CFG
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index b080f969fb9bc7..9958d6ea124f81 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -25,12 +25,15 @@ TEST(VPVerifierTest, VPInstructionUseBeforeDefSameBB) {
VPBB1->appendRecipe(DefI);
auto TC = std::make_unique<VPValue>();
+ VPBasicBlock *VPBB2 = new VPBasicBlock();
+ VPRegionBlock *R1 = new VPRegionBlock(VPBB2, VPBB2, "R1");
+ VPBlockUtils::connectBlocks(VPBB1, R1);
VPlan Plan(VPPH, &*TC, VPBB1);
#if GTEST_HAS_STREAM_REDIRECTION
::testing::internal::CaptureStderr();
#endif
- EXPECT_FALSE(VPlanVerifier::verifyPlanIsValid(Plan));
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
#if GTEST_HAS_STREAM_REDIRECTION
EXPECT_STREQ("Use before def!\n",
::testing::internal::GetCapturedStderr().c_str());
@@ -62,7 +65,7 @@ TEST(VPVerifierTest, VPInstructionUseBeforeDefDifferentBB) {
#if GTEST_HAS_STREAM_REDIRECTION
::testing::internal::CaptureStderr();
#endif
- EXPECT_FALSE(VPlanVerifier::verifyPlanIsValid(Plan));
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
#if GTEST_HAS_STREAM_REDIRECTION
EXPECT_STREQ("Use before def!\n",
::testing::internal::GetCapturedStderr().c_str());
@@ -97,6 +100,7 @@ TEST(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
VPBlockUtils::connectBlocks(VPBB3, VPBB4);
VPRegionBlock *R1 = new VPRegionBlock(VPBB2, VPBB4, "R1");
VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBB3->setParent(R1);
auto TC = std::make_unique<VPValue>();
VPlan Plan(VPPH, &*TC, VPBB1);
@@ -104,7 +108,7 @@ TEST(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
#if GTEST_HAS_STREAM_REDIRECTION
::testing::internal::CaptureStderr();
#endif
- EXPECT_FALSE(VPlanVerifier::verifyPlanIsValid(Plan));
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
#if GTEST_HAS_STREAM_REDIRECTION
EXPECT_STREQ("Use before def!\n",
::testing::internal::GetCapturedStderr().c_str());
@@ -112,4 +116,105 @@ TEST(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) {
delete Phi;
}
+
+TEST(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
+ VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
+ VPInstruction *BranchOnCond2 =
+ new VPInstruction(VPInstruction::BranchOnCond, {I1});
+
+ VPBasicBlock *VPPH = new VPBasicBlock("ph");
+ VPBasicBlock *VPBB1 = new VPBasicBlock();
+ VPBasicBlock *VPBB2 = new VPBasicBlock();
+
+ VPBB1->appendRecipe(I1);
+ VPBB1->appendRecipe(BranchOnCond2);
+ VPBB2->appendRecipe(CanIV);
+ VPBB2->appendRecipe(BranchOnCond);
+
+ VPRegionBlock *R1 = new VPRegionBlock(VPBB2, VPBB2, "R1");
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+
+ auto TC = std::make_unique<VPValue>();
+ VPlan Plan(VPPH, &*TC, VPBB1);
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("Multiple instances of the same successor.\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
+TEST(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
+ VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
+ auto *CanIV = new VPCanonicalIVPHIRecipe(I1, {});
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {CanIV});
+ VPInstruction *BranchOnCond2 =
+ new VPInstruction(VPInstruction::BranchOnCond, {I1});
+
+ VPBasicBlock *VPPH = new VPBasicBlock("ph");
+ VPBasicBlock *VPBB1 = new VPBasicBlock();
+ VPBasicBlock *VPBB2 = new VPBasicBlock();
+ VPBasicBlock *VPBB3 = new VPBasicBlock();
+
+ VPBB1->appendRecipe(I1);
+ VPBB2->appendRecipe(CanIV);
+ VPBB2->appendRecipe(BranchOnCond2);
+ VPBB3->appendRecipe(BranchOnCond);
+
+ VPBlockUtils::connectBlocks(VPBB2, VPBB3);
+ VPBlockUtils::connectBlocks(VPBB2, VPBB3);
+ VPRegionBlock *R1 = new VPRegionBlock(VPBB2, VPBB3, "R1");
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBB3->setParent(R1);
+
+ auto TC = std::make_unique<VPValue>();
+ VPlan Plan(VPPH, &*TC, VPBB1);
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("Multiple instances of the same successor.\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
+TEST(VPVerifierTest, BlockOutsideRegionWithParent) {
+ VPBasicBlock *VPPH = new VPBasicBlock("ph");
+ VPBasicBlock *VPBB1 = new VPBasicBlock();
+ VPBasicBlock *VPBB2 = new VPBasicBlock();
+
+ VPInstruction *DefI = new VPInstruction(Instruction::Add, {});
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {DefI});
+
+ VPBB1->appendRecipe(DefI);
+ VPBB2->appendRecipe(BranchOnCond);
+
+ VPRegionBlock *R1 = new VPRegionBlock(VPBB2, VPBB2, "R1");
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBB1->setParent(R1);
+
+ auto TC = std::make_unique<VPValue>();
+ VPlan Plan(VPPH, &*TC, VPBB1);
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("Predecessor is not in the same region.\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
} // namespace
More information about the llvm-commits
mailing list