[llvm] [VPlan] Consistently use (Part, 0) for first lane scalar values (PR #80271)

via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 15 05:35:57 PST 2024


ayalz wrote:

> > Perhaps it is the "other recipes" who should be fixed, to more consistently store their single scalar in per-part storage rather than lane zero? The current design of State is to hold UF per-part Values whenever possible, be they vectors or (uniform) scalars, and otherwise holds VF*UF per-lane Values, for every VPValue. The hasVectorValue() and hasScalarValue() indicators should better be renamed hasValuePerPart() and hasValuePerLane(), respectively.
> 
> That would be another option, but a potential problem could be recipes that only have their first lane used, but one of their operands would only define a vector value per-part. With this patch, this will work as accessing the per-lane value will create an extract from the per-part vector.
> 

Would it be better to stick with the current design, and fix such cases of missing extracts?

> > Unrolling loop-regions in VPlan by UF would simplify State to hold a single per-part Value or VF per-lane Values. Further unrolling replicating-regions and replicating-recipes by VF would simplify State to hold a single Value per VPValue.
> 
> Agreed, hopefully I should be able to share a patch for that by next week. It will require introducing additional VPInstructions operating on scalars, so this patch and #80273 should help to get us there.
> 
> > The current get() functions are overly complex, as they bridge gaps by packing VF scalar elements into a vector on-demand for a scalar producer to feed a vector consumer, by extracting VF scalar elements from a vector producer to feed a scalar-replicating consumer, and more. Introducing explicit pack and extract recipes in VPlan could simplify its code-gen and State, prior to unrollings.
> 
> That would be an alternative path, but I am quite close to getting ready to share unrolling as VPlan transformer, so I'd prefer to introduce explicit insert/extracts after explicit unrolling.

Sure, by all means; the comment was more future looking.

https://github.com/llvm/llvm-project/pull/80271


More information about the llvm-commits mailing list