[PATCH] D85794: [llvm][LV] Replace `unsigned VF` with `ElementCount VF` [NFCI]

[Renato Golin via Phabricator via llvm-commits]

rengolin accepted this revision.
rengolin added a comment.

I'm a bit wary of all the new asserts. The vectoriser is reasonable self-contained, and if you check for the scalable flag at entry points, you can assume safety inside.

But I'm not wary enough to go through the list of all possible entry-points. I think if the plan is to start implementing those pieces right away, then it could be fine as it for the time being.

I only have two small comments and LGTM. Thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6276
+  assert(!VF.Scalable && "scalable vectors not yet supported.");
+  if (VF.isScalar() && !VF.Scalable)
     return;
----------------
unnecessary test on `!VF.Scalable` already checked by assert.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6817
+  assert(!UserVF.Scalable && "scalable vectors not yet supported");
+  unsigned VF = UserVF.Min;
   // Outer loop handling: They may require CFG and instruction level
----------------
Name overloading, but now with different types. this will cause confusion.

Better to have `ElementCount VF = UserVF` and then use `VF.Min` locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85794