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

Tue Aug 24 02:35:47 PDT 2021

Ayal added a comment.

This follows-up on https://reviews.llvm.org/rGd995d63767624a60a5d3276f9f16d7b995435af1#inline-6235

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9489
+  // in ways that accessing values using original IR values is incorrect.
+  Plan->markIRToVPlanDisallowed();
+
----------------
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?

================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2149
+  /// is not safe any longer.
+  void markIRToVPlanDisallowed() { IRToVPlanAllowed = false; }
+
----------------
`disableGetVPValue()`?

Some related thoughts: should addVPValue() and removeVPValue() also be disallowed, i.e., disable all accesses to Value2VPValue? Conceptually, Value2VPValue can be destroyed, and potentially another type of VPlan which does not support Value2VPValue created and provided instead.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108573