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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1094
+                                                 bool Scalable) const {
+  if (Scalable) {
+    switch (RecKind) {
----------------
nit: bail out early to reduce indentation.
  if (!Scalable)
    return true;


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1111
+    }
+    return false;
+  }
----------------
nit: can be removed if you add the early bail out.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1522
+  bool canVectorizeReductions(ElementCount VF) {
+    for (auto &Reduction : Legal->getReductionVars()) {
+      RecurrenceDescriptor RdxDesc = Reduction.second;
----------------
nit: use `return llvm::all_of(....)` with lambda, instead of loop?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1525
+      if (!TTI.isLegalToVectorizeReduction(RdxDesc.getRecurrenceKind(),
+                                           VF.isScalable()))
+        return false;
----------------
Is it worth just passing the whole Recurrence descriptor and the whole of VF?

When passing the whole Recurrence descriptor, in the future the function can also determine whether it can vectorize an ordered reduction (e.g. ordered fadd) in the loop body using some instruction.


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

https://reviews.llvm.org/D95245