Page MenuHomePhabricator

[LV] Add VPValue operands to VPBlendRecipe (NFCI)

Authored by gilr on Apr 6 2020, 3:59 AM.



InnerLoopVectorizer's code called during VPlan execution still relies on original IR's def-use relations to decide which vector code to generate, limiting VPlan transformations ability to modify def-use relations and still have ILV generate the vector code.
This commit introduces VPValues for VPBlendRecipe to use as the values to blend. The recipe is generated with VPValues wrapping the phi's incoming values of the scalar phi. This reduces ingredient def-use usage by ILV as a step towards full VPlan-based def-use relations.

Diff Detail

Event Timeline

gilr created this revision.Apr 6 2020, 3:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2020, 3:59 AM
Ayal added a comment.Apr 7 2020, 2:10 AM

minor comments, looks good to me, thanks.


This matches current behavior, as an NFCI patch should.

Suggest as a follow-up to remove the assert and replace this with something like

VPValue *IncomingValue = Plan->getOrAddVPValue(Phi->getIncomingValue(In));
if (!EdgeMask)
  return new VPBlendRecipe(Phi, {IncomingValue}):

which assures that the number of operands will be either one or even (still need to assert it isn't 2), and fixes PR44800 as @fhahn commented.


Have the recipe provide interfaces for getNumIncomingValues(), getIncomingValue(predecessor#), getMask(predecessor#), hiding their interleaved storage layout?


"... an even number of operands"[, excluding 2]

gilr updated this revision to Diff 255711.Apr 7 2020, 10:15 AM
gilr marked 2 inline comments as done.

Address comments

Ayal accepted this revision.Apr 8 2020, 5:55 AM
Ayal added inline comments.

nit: better set NumIncoming = getNumIncomingValues() once and iterate until In < NumIncoming, as done below in print().

This revision is now accepted and ready to land.Apr 8 2020, 5:55 AM
gilr updated this revision to Diff 256005.Apr 8 2020, 6:30 AM
gilr marked an inline comment as done.

Addressed comments.

This revision was automatically updated to reflect the committed changes.


I wrote
for a crash that started appearing with this patch.