[PATCH] D107284: [LoopVectorize] Add support for replication of more intrinsics with scalable vectors

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8939-8940
+                                             bool IsScalable) const {
+  Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, TLI);
+  switch (ID) {
+  case Intrinsic::sideeffect:
----------------
nit: I think this can simply be:

  switch (CI->getIntrinsicID()) {


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8962
+    //      remove the call.
+    return IsScalable || OrigLoop->hasLoopInvariantOperands(CI);
+  default:
----------------
Perhaps not something to change in this patch, but given the change I've made in D107286, I wonder if it makes sense to mark any operation that has only loop-invariant operands as 'uniform' in `collectLoopUniforms`, e.g.

  if (all_of(I.operands(), [&](Value *V) { return isOutOfScope(V); }))
    addToWorklistIfAllowed(&I);

@fhahn any thoughts on this?

As a smaller first step, you could do the above suggestion only for this limited set of intrinsics (and then override IsUniform in handleReplication for this set of intrinsics if the VF is scalable).

That said, I don't really want to hold up this patch too much given that it fixes the issue sufficiently, and it's the last piece of the puzzle to build LNT and Clang with scalable vectorization enabled. So maybe such refactoring can be done in a follow-up patch?


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

https://reviews.llvm.org/D107284