[PATCH] D91718: [LV] Legalize scalable VF hints

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7442
+  if (!UserVF.isZero() &&
+      (UserVFIsLegal || (UserVF.isScalable() && MaxVF.isScalable()))) {
+    // FIXME: MaxVF is temporarily used inplace of UserVF for illegal scalable
----------------
david-arm wrote:
> Is this additional check necessary here? It looks like MaxVF came from computeMaXVF, which makes use of your clamping code above. If you revert this change do your tests still pass?
> Is this additional check necessary here?

Until the loop below supports scalable VFs yes, with this check we always fall into this block for a scalable VF, this handles the case where the UserVF isn't legal but a scalable VF is feasible so it clamps to the scalable MaxVF. Otherwise the assert below would be triggered, this is changed from the previous revision to implement @sdesmalen suggestion of: `UserVF = Scalable && MaxVF = Scalable && UserVF > MaxVF => Pick MaxVF`, rather than change MaxVF from scalable -> fixed.




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

https://reviews.llvm.org/D91718