This is an archive of the discontinued LLVM Phabricator instance.

[VPlan] Add VPValue::isDefinedOutsideVectorRegions helper (NFC).
ClosedPublic

Authored by fhahn on Sep 11 2022, 4:46 AM.

Details

Summary

@Ayal suggested a better named helper than using !getDef() to check if
a value is invariant across all parts.

The property we are using here is that the VPValue is defined outside
any vector loop region. There's a TODO left to handle recipes defined in
pre-header blocks.

Diff Detail

Event Timeline

fhahn created this revision.Sep 11 2022, 4:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2022, 4:46 AM
fhahn requested review of this revision.Sep 11 2022, 4:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2022, 4:46 AM
reames requested changes to this revision.Sep 12 2022, 8:07 AM
reames added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
3057–3058

With this placement, this is equivalent to return false. The interesting case is the return true below.

Maybe:

if (VPV->isDefinedOutsideVectorRegion())
  return true;
auto *Def = VPV->getDef();
assert(Def);  
...
This revision now requires changes to proceed.Sep 12 2022, 8:07 AM
Ayal added inline comments.Sep 13 2022, 6:44 AM
llvm/lib/Transforms/Vectorize/VPlanValue.h
205

Thanks for following up!
Perhaps something like "isaVPLiveIn()" analogous to isa<VPLiveOut>()? Both are saying it's a wrapper of an IR value/user outside the scope of VPlan rather than a recipe inside this scope.
More precise type information could be derived from the wrapped underlying IR, or implied assuming that VPlan is only fed-by and feeds scalars from/to IR.

fhahn updated this revision to Diff 468156.Oct 17 2022, 4:14 AM

Reorder code as suggested, apologies for the delay

fhahn marked 2 inline comments as done.Oct 17 2022, 4:17 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
3057–3058

Thanks, re-ordered the code!

llvm/lib/Transforms/Vectorize/VPlanValue.h
205

The helper is intended to go beyond pure live-ins in the future. In particular, the current definition could also return true for recipes defined in the pre-header, as in the code below. I wasn't able to come up with a test case to trigger this code though for now. Does that make sense?

+bool VPValue::isDefinedOutsideVectorRegions() const {
+  const VPRecipeBase *Def = getDef();
+  if (!Def)
+    return true;
+  const VPBasicBlock *DefBlock = Def->getParent();
+  const VPlan &Plan = *DefBlock->getPlan();
+  return Plan.getEntry() == DefBlock;
+}
+
fhahn retitled this revision from pVplan] Add VPValue::isDefinedOutsideVectorRegions helper (NFC). to [VPlan] Add VPValue::isDefinedOutsideVectorRegions helper (NFC)..Oct 17 2022, 4:17 AM
reames accepted this revision.Oct 17 2022, 9:25 AM

LGTM

This revision is now accepted and ready to land.Oct 17 2022, 9:25 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked 2 inline comments as done.