[VPlan] Add narrowToSingleScalarRecipe transform. #139150
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1084,6 +1084,40 @@ void VPlanTransforms::simplifyRecipes(VPlan &Plan, Type &CanonicalIVTy) { | |||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| static void convertToUniformRecipes(VPlan &Plan) { | ||||||||||||||||||||
| auto TryToNarrow = [](VPBasicBlock *VPBB) { | ||||||||||||||||||||
| for (VPRecipeBase &R : make_early_inc_range(reverse(*VPBB))) { | ||||||||||||||||||||
| // Try to narrow wide and replicating recipes to uniform recipes, based on | ||||||||||||||||||||
| // VPlan analysis. | ||||||||||||||||||||
| auto *Def = dyn_cast<VPSingleDefRecipe>(&R); | ||||||||||||||||||||
| if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) || | ||||||||||||||||||||
| !Def->getUnderlyingValue()) | ||||||||||||||||||||
|
||||||||||||||||||||
| !Def->getUnderlyingValue()) |
can be asserted instead if desired - these recipes must have underlying values - in order to know what to replicate or widen.
There was a problem hiding this comment.
It could be asserted for VPReplicateRecipe, but not for VPWidenRecipe which does not require a underlying value.
There was a problem hiding this comment.
Hmm, if VPWidenRecipe uses the (optional) underlying value only for metadata and FMF, could it be replaced with VPInstruction?
There was a problem hiding this comment.
Yep we should be able to do that soon. I'll give it a try, need to see if we rely on the encoded facts that certain recipes are widened in various places.
Outdated
There was a problem hiding this comment.
| auto *Def = dyn_cast<VPSingleDefRecipe>(&R); | |
| if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) || | |
| !Def->getUnderlyingValue()) | |
| continue; |
There was a problem hiding this comment.
| auto *RepR = dyn_cast<VPReplicateRecipe>(&R); | |
| if (RepR && RepR->isUniform()) | |
| continue; | |
| auto *RepR = dyn_cast<VPReplicateRecipe>(&R); | |
| if (!RepR && !isa<VPWidenRecipe>(&R)) | |
| continue; | |
| if (RepR && RepR->isUniform()) | |
| continue; |
There was a problem hiding this comment.
| auto *SingleDef = cast<VPSingleDefRecipe>(&R); |
or RepOrWidenR.
Outdated
There was a problem hiding this comment.
| // Skip recipes that aren't uniform and don't have only their scalar | |
| // Skip recipes that aren't single scalar or don't have only their scalar |
There was a problem hiding this comment.
The and here should be accurate, it skips cases that have non-scalar uses, as this may require introducing broadcasts. This is something that will be generalized in the future.
There was a problem hiding this comment.
Ah yes, was thinking about the recipes we process below, updated, thanks
Outdated
There was a problem hiding this comment.
| // results used. In the later case, we would introduce extra broadcasts. | |
| // results used. In the latter case, we would introduce extra broadcasts. |
Outdated
There was a problem hiding this comment.
The term "UniformAfterVectorization" and VPReplicateRecipe's "uniform" field should be renamed. Being uniform (having same value for all lanes) is independent of being before or after vectorization. The term stands for "singleLane" or "singleScalar", which is typically associated with the first lane (as in unit-stride, i.e., clearly non-uniform, GEPs whose first lane is the only one used), and with all lanes when the value is uniform.
There was a problem hiding this comment.
Agreed, this is long overdue! #140134
Outdated
There was a problem hiding this comment.
Is there a particular reason we're using a lambda here?
There was a problem hiding this comment.
As if this pass had a class of its own where its main runPass() called a TryToNarrow() method on each basic block, as in runOnBasicBlock(), independently (and conceptually in parallel).
There was a problem hiding this comment.
I have some follow-up patches that may calls this on additional blocks, which was why I had the lambda originally. Inlined for now, thanks
Outdated
There was a problem hiding this comment.
Do we need to apply uniform analysis if VF=1? If not, could we skip it when VF=1?
There was a problem hiding this comment.
Hmm, suspect so, given that in such case all replicate recipes should already be "uniform" and widen recipes are irrelevant.
There was a problem hiding this comment.
Yep, added a check isScalarVFOnly() to the transform
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.
as this captures both uniformity and only-first-lane-used? Also affects title of patch.
Analogous to
truncateToMinimalBitwidths()which aims to reduce each lane to fewer bits, this aims to reduce each part to fewest lanes - to one. Perhaps both should start withnarrow, as used in the now inlined lambda.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.
Updated to narrowToSingleScalarRecipe, thanks