[PATCH] D102394: [LoopVectorize] Don't attempt to widen certain calls for scalable vectors

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:665
+  /// a given ElementCount VF.
+  bool isLegalScalableVectorIntrinsic(Intrinsic::ID IID) const;
+
----------------
nit: I read `isLegalScalableVectorIntrinsic` as asking if the "scalable vector intrinsic" is legal as if the behaviour of the intrinsic is specific to scalable vectors. Rather, what you're asking is if the intrinsic (not specific to scalable vectors) is legal to use with scalable vector arguments, so from that reasoning I prefer `isIntrinsicLegalForScalableVectors`.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5661
+  for (BasicBlock *BB : TheLoop->blocks()) {
+    for (Instruction &I : *BB) {
+      if (auto *CI = dyn_cast<CallInst>(&I)) {
----------------
When debug-info is enabled, that will insert llvm.dbg intrinsic calls which aren't handled in your switch, but maybe it's better to avoid getting into that to begin with by discarding instructions where `I.isDebugOrPseudoInst() == true`.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5674
+        if (VFDatabase::getMappings(*CI).empty()) {
+          Msg = "Scalable vectorization not supported for the call "
+                "instructions found in this loop";
----------------
Instead of passing in Msg, can you just call `reportVectorizationInfo` here? Also, can you make the message more specific by telling which function is not vectorizable with scalable vectors?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5676
+                "instructions found in this loop";
+          return false;
+        }
----------------
Shouldn't the code traverse the mappings in the VFdatabase to see if there is a scalable form available?


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

https://reviews.llvm.org/D102394