[PATCH] D108573: [VPlan] Introduce code to limit querying VPValues using IR references.

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 25 08:52:47 PDT 2021


Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

This looks good to me, thanks for following-up!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9489
+  // in ways that accessing values using original IR values is incorrect.
+  Plan->markIRToVPlanDisallowed();
+
----------------
fhahn wrote:
> Ayal wrote:
> > Perhaps refactor to have buildVPlanWithVPRecipes() take care of building the initial unoptimized VPlan only, ending with marking IRToVPlan disallowed. I.e., invoke sinkScalarOperands() and mergeReplicateRegion() optimizations after buildVPlanWithVPRecipes() returns?
> I'm not sure, introduction of `FirstOrderRecurrenceSplice` could also be moved after disallowing Value2VPValue access, but it is not an optimization. Should we do the potential split separately, to keep the diff smaller?
Sure, refactoring can be done separately. And indeed may include non-optimization stages that are pure VPlan2VPlan transformations.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2193
+  /// OverrideAllowed can be used to disable checking whether it is safe to
+  /// query VPValues using IR Values.
+  VPValue *getOrAddVPValue(Value *V, bool OverrideAllowed = false) {
----------------
Note that calling getOrAddVPValue() with OverrideAllowed allows to get a VPValue but not add one. This is fine, as the override is temporary anyhow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108573/new/

https://reviews.llvm.org/D108573



More information about the llvm-commits mailing list