diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h index 2c4cac7655ec9..5fd7a369bf735 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -1135,7 +1135,9 @@ class VPPhiAccessors { const VPBasicBlock *getIncomingBlock(unsigned Idx) const; /// Returns the number of incoming values, also number of incoming blocks. - unsigned getNumIncoming() const { return getAsRecipe()->getNumOperands(); } + virtual unsigned getNumIncoming() const { + return getAsRecipe()->getNumOperands(); + } #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) /// Print the recipe. @@ -1234,7 +1236,7 @@ class VPIRInstruction : public VPRecipeBase { /// cast/dyn_cast/isa and execute() implementation. A single VPValue operand is /// allowed, and it is used to add a new incoming value for the single /// predecessor VPBB. -struct VPIRPhi : public VPIRInstruction { +struct VPIRPhi : public VPIRInstruction, public VPPhiAccessors { VPIRPhi(PHINode &PN) : VPIRInstruction(PN) {} static inline bool classof(const VPRecipeBase *U) { @@ -1251,6 +1253,9 @@ struct VPIRPhi : public VPIRInstruction { void print(raw_ostream &O, const Twine &Indent, VPSlotTracker &SlotTracker) const override; #endif + +protected: + const VPRecipeBase *getAsRecipe() const override { return this; } }; /// Helper to manage IR metadata for recipes. It filters out metadata that @@ -1785,13 +1790,15 @@ class VPVectorPointerRecipe : public VPRecipeWithIRFlags, /// * VPWidenPointerInductionRecipe: Generate vector and scalar values for a /// pointer induction. Produces either a vector PHI per-part or scalar values /// per-lane based on the canonical induction. -class VPHeaderPHIRecipe : public VPSingleDefRecipe { +class VPHeaderPHIRecipe : public VPSingleDefRecipe, public VPPhiAccessors { protected: VPHeaderPHIRecipe(unsigned char VPDefID, Instruction *UnderlyingInstr, VPValue *Start, DebugLoc DL = {}) : VPSingleDefRecipe(VPDefID, ArrayRef({Start}), UnderlyingInstr, DL) { } + const VPRecipeBase *getAsRecipe() const override { return this; } + public: ~VPHeaderPHIRecipe() override = default; @@ -1980,6 +1987,11 @@ class VPWidenIntOrFpInductionRecipe : public VPWidenInductionRecipe { return isUnrolled() ? getOperand(getNumOperands() - 2) : nullptr; } + /// Returns the number of incoming values, also number of incoming blocks. + /// Note that at the moment, VPWidenIntOrFpInductionRecipes only have a single + /// incoming value, its start value. + unsigned getNumIncoming() const override { return 1; } + /// Returns the first defined value as TruncInst, if it is one or nullptr /// otherwise. TruncInst *getTruncInst() { return Trunc; } @@ -3283,6 +3295,46 @@ class VPScalarIVStepsRecipe : public VPRecipeWithIRFlags, } }; +/// Casting from VPRecipeBase -> VPPhiAccessors is supported for all recipe +/// types implementing VPPhiAccessors. Used by isa<> & co. +template <> struct CastIsPossible { + static inline bool isPossible(const VPRecipeBase *f) { + // TODO: include VPPredInstPHIRecipe too, once it implements VPPhiAccessors. + return isa(f); + } +}; +/// Support casting from VPRecipeBase -> VPPhiAccessors, by down-casting to the +/// recipe types implementing VPPhiAccessors. Used by cast<>, dyn_cast<> & co. +template <> +struct CastInfo + : public CastIsPossible { + + using Self = CastInfo; + + /// doCast is used by cast<>. + static inline VPPhiAccessors *doCast(const VPRecipeBase *R) { + return const_cast([R]() -> const VPPhiAccessors * { + switch (R->getVPDefID()) { + case VPDef::VPInstructionSC: + return cast(R); + case VPDef::VPIRInstructionSC: + return cast(R); + case VPDef::VPWidenPHISC: + return cast(R); + default: + return cast(R); + } + }()); + } + + /// doCastIfPossible is used by dyn_cast<>. + static inline VPPhiAccessors *doCastIfPossible(const VPRecipeBase *f) { + if (!Self::isPossible(f)) + return nullptr; + return doCast(f); + } +}; + /// VPBasicBlock serves as the leaf of the Hierarchical Control-Flow Graph. It /// holds a sequence of zero or more VPRecipe's each representing a sequence of /// output IR instructions. All PHI-like recipes must come before any non-PHI recipes. diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp index b8205545a4f5e..1e7e039a18d56 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp @@ -192,8 +192,7 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) { if (!verifyPhiRecipes(VPBB)) return false; - // Verify that defs in VPBB dominate all their uses. The current - // implementation is still incomplete. + // Verify that defs in VPBB dominate all their uses. DenseMap RecipeNumbering; unsigned Cnt = 0; for (const VPRecipeBase &R : *VPBB) @@ -220,12 +219,31 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) { for (const VPUser *U : V->users()) { auto *UI = cast(U); - // TODO: check dominance of incoming values for phis properly. - if (!UI || - isa(UI) || - (isa(UI) && - cast(UI)->getOpcode() == Instruction::PHI)) + if (auto *Phi = dyn_cast(UI)) { + for (unsigned Idx = 0; Idx != Phi->getNumIncoming(); ++Idx) { + VPValue *IncomingVPV = Phi->getIncomingValue(Idx); + if (IncomingVPV != V) + continue; + + const VPBasicBlock *IncomingVPBB = Phi->getIncomingBlock(Idx); + if (VPDT.dominates(VPBB, IncomingVPBB)) + continue; + + errs() << "Incoming def at index " << Idx + << " does not dominate incoming block!\n"; +#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) + VPSlotTracker Tracker(VPBB->getPlan()); + IncomingVPV->getDefiningRecipe()->print(errs(), " ", Tracker); + errs() << "\n does not dominate " << IncomingVPBB->getName() + << " for\n"; + UI->print(errs(), " ", Tracker); +#endif + return false; + } + continue; + } + // TODO: Also verify VPPredInstPHIRecipe. + if (isa(UI)) continue; // If the user is in the same block, check it comes after R in the diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp index 84b7e33146811..9e0196c44f441 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp @@ -143,6 +143,43 @@ TEST_F(VPVerifierTest, VPBlendUseBeforeDefDifferentBB) { delete Phi; } +TEST_F(VPVerifierTest, VPPhiIncomingValueDoesntDominateIncomingBlock) { + VPlan &Plan = getPlan(); + IntegerType *Int32 = IntegerType::get(C, 32); + VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 0)); + + VPBasicBlock *VPBB1 = Plan.getEntry(); + VPBasicBlock *VPBB2 = Plan.createVPBasicBlock(""); + VPBasicBlock *VPBB3 = Plan.createVPBasicBlock(""); + + VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero}); + VPPhi *Phi = new VPPhi({DefI}, {}); + VPBB2->appendRecipe(Phi); + VPBB2->appendRecipe(DefI); + auto *CanIV = new VPCanonicalIVPHIRecipe(Zero, {}); + VPBB3->appendRecipe(CanIV); + + VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB3, VPBB3, "R1"); + VPBlockUtils::connectBlocks(VPBB1, VPBB2); + VPBlockUtils::connectBlocks(VPBB2, R1); +#if GTEST_HAS_STREAM_REDIRECTION + ::testing::internal::CaptureStderr(); +#endif + EXPECT_FALSE(verifyVPlanIsValid(Plan)); +#if GTEST_HAS_STREAM_REDIRECTION +#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) + EXPECT_STREQ("Incoming def at index 0 does not dominate incoming block!\n" + " EMIT vp<%2> = add ir<0>\n" + " does not dominate preheader for\n" + " EMIT vp<%1> = phi [ vp<%2>, preheader ]", + ::testing::internal::GetCapturedStderr().c_str()); +#else + EXPECT_STREQ("Use before def!\n", + ::testing::internal::GetCapturedStderr().c_str()); +#endif +#endif +} + TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) { VPlan &Plan = getPlan(); VPValue *Zero = Plan.getOrAddLiveIn(ConstantInt::get(Type::getInt32Ty(C), 0));