[VPlan] Add VPExpressionRecipe, replacing extended reduction recipes. #144281
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -525,14 +525,13 @@ class VPSingleDefRecipe : public VPRecipeBase, public VPValue { | |||||||
|
|
||||||||
| static inline bool classof(const VPRecipeBase *R) { | ||||||||
| switch (R->getVPDefID()) { | ||||||||
| case VPRecipeBase::VPBundleSC: | ||||||||
| case VPRecipeBase::VPDerivedIVSC: | ||||||||
| case VPRecipeBase::VPEVLBasedIVPHISC: | ||||||||
| case VPRecipeBase::VPExpandSCEVSC: | ||||||||
| case VPRecipeBase::VPInstructionSC: | ||||||||
| case VPRecipeBase::VPReductionEVLSC: | ||||||||
| case VPRecipeBase::VPReductionSC: | ||||||||
| case VPRecipeBase::VPReplicateSC: | ||||||||
| case VPRecipeBase::VPScalarIVStepsSC: | ||||||||
| case VPRecipeBase::VPVectorPointerSC: | ||||||||
|
|
@@ -852,9 +851,7 @@ struct VPRecipeWithIRFlags : public VPSingleDefRecipe, public VPIRFlags { | |||||||
| R->getVPDefID() == VPRecipeBase::VPReductionEVLSC || | ||||||||
| R->getVPDefID() == VPRecipeBase::VPReplicateSC || | ||||||||
| R->getVPDefID() == VPRecipeBase::VPVectorEndPointerSC || | ||||||||
| R->getVPDefID() == VPRecipeBase::VPVectorPointerSC; | ||||||||
| } | ||||||||
|
|
||||||||
| static inline bool classof(const VPUser *U) { | ||||||||
|
|
@@ -2432,28 +2429,6 @@ class VPReductionRecipe : public VPRecipeWithIRFlags { | |||||||
| setUnderlyingValue(I); | ||||||||
| } | ||||||||
|
|
||||||||
| public: | ||||||||
| VPReductionRecipe(RecurKind RdxKind, FastMathFlags FMFs, Instruction *I, | ||||||||
| VPValue *ChainOp, VPValue *VecOp, VPValue *CondOp, | ||||||||
|
|
@@ -2479,9 +2454,7 @@ class VPReductionRecipe : public VPRecipeWithIRFlags { | |||||||
|
|
||||||||
| static inline bool classof(const VPRecipeBase *R) { | ||||||||
| return R->getVPDefID() == VPRecipeBase::VPReductionSC || | ||||||||
| R->getVPDefID() == VPRecipeBase::VPReductionEVLSC; | ||||||||
| } | ||||||||
|
|
||||||||
| static inline bool classof(const VPUser *U) { | ||||||||
|
|
@@ -2620,190 +2593,6 @@ class VPReductionEVLRecipe : public VPReductionRecipe { | |||||||
| } | ||||||||
| }; | ||||||||
|
|
||||||||
| /// VPReplicateRecipe replicates a given instruction producing multiple scalar | ||||||||
| /// copies of the original scalar type, one per lane, instead of producing a | ||||||||
| /// single copy of widened type for all lanes. If the instruction is known to be | ||||||||
|
|
@@ -2922,6 +2711,127 @@ class VPBranchOnMaskRecipe : public VPRecipeBase { | |||||||
| } | ||||||||
| }; | ||||||||
|
|
||||||||
| /// A recipe to combine multiple recipes into a 'bundle' recipe, which should be | ||||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. The recipes aren't bundled until they're in the recipe so I don't think saying "multiple 'bundled' recipes" is correct. The original comment is correct in that the recipes are separate until they're bundled into this class. |
||||||||
| /// considered as single entity for cost-modeling and transforms. The recipe | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| /// needs to be 'unbundled', i.e. replaced by its individual recipes before | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| /// execute. | ||||||||
| class VPBundleRecipe : public VPSingleDefRecipe { | ||||||||
|
There was a problem hiding this comment. One could bundle recipes that together provide multiple defs, such as interleaved loads. Perhaps more accurately called VPSingleDefBundleRecipe, or There was a problem hiding this comment. Updated to |
||||||||
| enum class BundleTypes { | ||||||||
| ExtendedReduction, | ||||||||
|
There was a problem hiding this comment. So should this be
Suggested change
? |
||||||||
| MulAccumulateReduction, | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| }; | ||||||||
|
|
||||||||
| /// Recipes bundled together in this VPBundleRecipe. | ||||||||
| SmallVector<VPSingleDefRecipe *> BundledOps; | ||||||||
|
|
||||||||
| /// Temporary VPValues used for external operands of the bundle, i.e. operands | ||||||||
| /// not defined by recipes in the bundle. | ||||||||
|
There was a problem hiding this comment. These are placeholder VPValues defined temporarily by the bundle recipe itself to feed its bundled recipes. Note that the VPBundleRecipe, being a single def VPValue, feeds out-of bundle recipes itself. |
||||||||
| SmallVector<VPValue *> TmpValues; | ||||||||
|
There was a problem hiding this comment.
Suggested change
? |
||||||||
|
|
||||||||
| /// Type of the bundle. | ||||||||
| BundleTypes BundleType; | ||||||||
|
There was a problem hiding this comment. Better placed next to the enum definition above, documenting each? There was a problem hiding this comment. Moved the definition down here, thanks |
||||||||
|
|
||||||||
|
There was a problem hiding this comment. Worth documenting, especially \p Operands - perhaps better called CloneOperands, perhaps defined to be optional, passed only to save time? Does it really save anything. There was a problem hiding this comment. Operands is gone now, moved logic from bundle() directly to the constructor, thanks |
||||||||
| VPBundleRecipe(BundleTypes BundleType, ArrayRef<VPSingleDefRecipe *> ToBundle) | ||||||||
| : VPSingleDefRecipe(VPDef::VPBundleSC, {}, {}), BundledOps(ToBundle), | ||||||||
| BundleType(BundleType) { | ||||||||
| // Bundle up the operand recipes. | ||||||||
| SmallPtrSet<VPUser *, 4> BundledUsers; | ||||||||
| for (auto *R : BundledOps) | ||||||||
| BundledUsers.insert(R); | ||||||||
|
|
||||||||
| // Recipes in the bundle, except the last one, must only be used inside the | ||||||||
| // bundle. If there other external users, clone the recipes for the bundle. | ||||||||
| for (const auto &[Idx, R] : enumerate(drop_end(ToBundle))) { | ||||||||
| if (all_of(R->users(), [&BundledUsers](VPUser *U) { | ||||||||
| return BundledUsers.contains(U); | ||||||||
| })) { | ||||||||
| if (R->getParent()) | ||||||||
| R->removeFromParent(); | ||||||||
| continue; | ||||||||
| } | ||||||||
| // The users external to the bundle. Clone the recipe for use in the | ||||||||
| // bundle and update all its in-bundle users. | ||||||||
| VPSingleDefRecipe *Copy = R->clone(); | ||||||||
| BundledOps[Idx] = Copy; | ||||||||
| BundledUsers.insert(Copy); | ||||||||
| R->replaceUsesWithIf(Copy, [&BundledUsers](VPUser &U, unsigned) { | ||||||||
| return BundledUsers.contains(&U); | ||||||||
| }); | ||||||||
| } | ||||||||
| BundledOps.back()->removeFromParent(); | ||||||||
|
There was a problem hiding this comment. While experimenting with rebasing #136173 on top of this, I've found that the new bundle recipe can't be used when interleaving. This line for example needs to be gated behind EDIT: I've done some more digging and it could actually be an issue specific to partial reductions. There was a problem hiding this comment. It looks like we were missing some test coverage for interleaving VPExtendedReductions/VPMulAccumulateReductions. The issue was that we didn't clone the recipes in the bundle when cloning the VPBundleRecipe. Updated and now it should work fine with interleaving (tested in There was a problem hiding this comment. Great, thanks for that. |
||||||||
|
|
||||||||
| // Internalize all external operands to the bundled operations. To do so, | ||||||||
| // create new temporary VPValues for all operands not defined by recipe in | ||||||||
| // the bundle. The original operands are added as operands of the | ||||||||
| // VPBundleRecipe. | ||||||||
| for (auto *R : BundledOps) { | ||||||||
| for (const auto &[Idx, Op] : enumerate(R->operands())) { | ||||||||
| auto *Def = Op->getDefiningRecipe(); | ||||||||
| if (Def && BundledUsers.contains(Def)) | ||||||||
| continue; | ||||||||
| addOperand(Op); | ||||||||
| TmpValues.push_back(new VPValue()); | ||||||||
| R->setOperand(Idx, TmpValues.back()); | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| public: | ||||||||
| VPBundleRecipe(VPWidenCastRecipe *Ext, VPReductionRecipe *Red) | ||||||||
| : VPBundleRecipe(BundleTypes::ExtendedReduction, {Ext, Red}) {} | ||||||||
| VPBundleRecipe(VPWidenRecipe *Mul, VPReductionRecipe *Red) | ||||||||
| : VPBundleRecipe(BundleTypes::MulAccumulateReduction, {Mul, Red}) {} | ||||||||
| VPBundleRecipe(VPWidenCastRecipe *Ext0, VPWidenCastRecipe *Ext1, | ||||||||
| VPWidenRecipe *Mul, VPReductionRecipe *Red) | ||||||||
| : VPBundleRecipe(BundleTypes::MulAccumulateReduction, | ||||||||
|
There was a problem hiding this comment. Can be defined as ExtMulAccumulateReduction? There was a problem hiding this comment. Updated the type name of the enum value |
||||||||
| {Ext0, Ext1, Mul, Red}) {} | ||||||||
| VPBundleRecipe(VPWidenCastRecipe *Ext0, VPWidenCastRecipe *Ext1, | ||||||||
| VPWidenRecipe *Mul, VPWidenCastRecipe *Ext2, | ||||||||
| VPReductionRecipe *Red) | ||||||||
| : VPBundleRecipe(BundleTypes::MulAccumulateReduction, | ||||||||
|
There was a problem hiding this comment. Can be defined as ExtMulExtAccumulateReduction? |
||||||||
| {Ext0, Ext1, Mul, Ext2, Red}) {} | ||||||||
|
|
||||||||
| ~VPBundleRecipe() override { | ||||||||
| SmallPtrSet<VPRecipeBase *, 4> Seen; | ||||||||
| for (auto *R : reverse(BundledOps)) | ||||||||
| if (Seen.insert(R).second) | ||||||||
| delete R; | ||||||||
| for (VPValue *T : TmpValues) | ||||||||
| delete T; | ||||||||
| } | ||||||||
|
|
||||||||
| VP_CLASSOF_IMPL(VPDef::VPBundleSC) | ||||||||
|
|
||||||||
| VPBundleRecipe *clone() override { | ||||||||
| assert(!BundledOps.empty() && "empty bundles should be removed"); | ||||||||
| return new VPBundleRecipe(BundleType, BundledOps); | ||||||||
|
There was a problem hiding this comment. What happens if we call this after calling There was a problem hiding this comment. This shouldn't happen, I added an assert. |
||||||||
| } | ||||||||
|
|
||||||||
| /// Return the VPSingleDefRecipe producing the final result of the bundled | ||||||||
| /// recipe. | ||||||||
|
There was a problem hiding this comment. Without loss of generality this can be the last bundled recipe. |
||||||||
| VPSingleDefRecipe *getResultOp() const { return BundledOps.back(); } | ||||||||
|
|
||||||||
| /// Insert the bundled recipes back into the VPlan, directly before the | ||||||||
| /// current recipe. Leaves the bundle recipe empty and the recipe must be | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| /// removed before codegen. | ||||||||
| void unbundle(); | ||||||||
|
|
||||||||
|
|
||||||||
| /// Generate the extraction of the appropriate bit from the block mask and the | ||||||||
| /// conditional branch. | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| void execute(VPTransformState &State) override { | ||||||||
| llvm_unreachable("recipe must be removed before execute"); | ||||||||
| } | ||||||||
|
|
||||||||
| InstructionCost computeCost(ElementCount VF, | ||||||||
| VPCostContext &Ctx) const override; | ||||||||
|
|
||||||||
| #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||||||||
| /// Print the recipe. | ||||||||
| void print(raw_ostream &O, const Twine &Indent, | ||||||||
| VPSlotTracker &SlotTracker) const override; | ||||||||
| #endif | ||||||||
| }; | ||||||||
|
|
||||||||
| /// VPPredInstPHIRecipe is a recipe for generating the phi nodes needed when | ||||||||
| /// control converges back from a Branch-on-Mask. The phi nodes are needed in | ||||||||
| /// order to merge values that are set under such a branch and feed their uses. | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be consistent, and place below following lex order.
But would
VPExpressionRecipebe better - conveying the notion that (a) it produces a single value (b) out of multiple simpler components - which are potentially side-effect-free(?) and potentially (sub)expressions themselves(?). I.e., a bundle recipe can potentially be bundled inside another bundle recipe, unbundle() can be recursive. Worth commenting that this may be possible but that currently bundle recipes are restricted to flat/single-level bundling.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is
VPIdiomRecipeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated to VPExpressionRecipe, which is also slightly more compact. Tried to update various places refering to bundle as appropriate, just keeping
unbundle.