[PATCH] D115651: [LV] Enable scalable vectorization by default for SVE cores.

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:171
+  /// scalable vectors when the cost-model is inconclusive. This is the
+  /// default when the scalable.enable hint is enabled through a pragma.
+  PreferScalable = 1,
----------------
Given we've moved this out of LoopVectorize.cpp I wonder if it's better not to mention the hints?


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:1410
+  /// \return the preferred style of vectorization.
+  ScalableVectorizationKind getPreferredScalableVectorizationKind() const;
+
----------------
Given the class name is `ScalableVectorizationKind ` perhaps it's more consistent to name the function `getScalableVectorizationKind()` instead, i.e. like in other places where we have things like get'ClassName' or get'Enum'.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:102
+      const Loop *L, bool InterleaveOnlyWhenForced,
+      ScalableVectorizationKind PreferredScalableVectorizationKind,
+      OptimizationRemarkEmitter &ORE);
----------------
Hi @sdesmalen, I personally find it a little wordy to have a variable called PreferredScalableVectorizationKind that itself has enum values with the word Preferred in it, i.e. PreferScalable, etc. Also, one of the kinds is Unspecified, which is not really a statement of preference. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115651