[PATCH] D95245: [SVE] Add support for scalable vectorization of loops with int/fast FP reductions

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

LV changes LGTM, thanks for the updates!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1528
+  /// Returns true if the target machine supports all of the reduction
+  /// variables found for the given VF
+  bool canVectorizeReductions(ElementCount VF) {
----------------
nit: `.` at end of sentence.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1530
+  bool canVectorizeReductions(ElementCount VF) {
+    return (llvm::all_of(Legal->getReductionVars(), [&](auto &Reduction) -> bool {
+      RecurrenceDescriptor RdxDesc = Reduction.second;
----------------
nit: `llvm::` should not be required


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5701
+                           "Using fixed-width vectorization instead.\n");
+      ORE->emit([&]() {
+        return OptimizationRemarkAnalysis(DEBUG_TYPE, "ScalableVFUnfeasible",
----------------
I think you should be bale to use `reportVectorizationFailure` to print to `dbgs()` and generate a remark with the same message


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-reductions.ll:20
+entry:
+  %cmp6 = icmp sgt i64 %n, 0
+  br i1 %cmp6, label %for.body, label %for.end
----------------
nit: those checks should not be needed.


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

https://reviews.llvm.org/D95245