Page MenuHomePhabricator

[VPlan] Support extracting lanes for defs managed in VPTransformState.
ClosedPublic

Authored by fhahn on May 29 2020, 4:18 AM.

Details

Summary

Currently extracting a lane for a VPValue def is not supported, if it is
managed directly by VPTransformState (e.g. because it is created by a
VPInstruction or an external VPValue def).

For now, simply extract the requested lane. In the future, we should
also cache the extracted scalar values, similar to LV.

Diff Detail

Event Timeline

fhahn created this revision.May 29 2020, 4:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2020, 4:18 AM
SjoerdMeijer added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
283

Probably the comment about falling back to ILV now needs to be placed here?

And perhaps a brief comment added to the lines/comments above, what the reason could be for a Value not having a Def (at least I would be curious about that)?

fhahn updated this revision to Diff 267899.Jun 2 2020, 8:43 AM

Adjust comments, clarify which Defs are directly managed by VPTransformState and that we delegate to ILV for others.

SjoerdMeijer accepted this revision.EditedJun 2 2020, 8:59 AM

Thanks for this. I have played quite a bit with this patch (in the context of D79100), and this behaves entirely as expected. The change is minimal/straightforward, so LGTM.

llvm/lib/Transforms/Vectorize/VPlan.h
278

Really minor nit: perhaps "For those" -> "For those Defs"
(think that reads a bit easier)

This revision is now accepted and ready to land.Jun 2 2020, 8:59 AM
Ayal added inline comments.Jun 2 2020, 11:55 AM
llvm/lib/Transforms/Vectorize/VPlan.h
281

This should in general also support scalar Values set per part, i.e., invariants.
Going forward, better model extracts and inserts upfront in VPlan, rather than generating them on-demand during code-gen.

fhahn marked an inline comment as done.Jun 2 2020, 1:18 PM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
281

Going forward, better model extracts and inserts upfront in VPlan, rather than generating them on-demand during code-gen.

Sounds good! I'll land the patch as-is tomorrow as a first step, unless there's additional feedback.

This revision was automatically updated to reflect the committed changes.