[PATCH] D142894: [LoopVectorize] Use overflow-check analysis to improve tail-folding.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3500
+    if (VF.isScalable()) {
+      if (std::optional<unsigned> MaxVScale = TTI.getMaxVScale())
+        MaxVFElts *= *MaxVScale;
----------------
paulwalker-arm wrote:
> We so need to get rid of `TTI::getMaxVScale()`.  I think function attributes should take precedence but then this is the order used within `getMaxLegalScalableVF` so perhaps best fixed separately. Up to you.
I'd rather fix that separately so that the order is consistent between getMaxLegalScalableVF and here.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8970
+       ElementCount::isKnownLT(VF, Range.End); VF *= 2)
+    IVUpdateCannotOverflow &= CM.isIndvarOverflowCheckKnownFalse(VF);
+
----------------
paulwalker-arm wrote:
> Is `addCanonicalIVRecipes` being passed the "wrong" tail folding style a functional issue? or just a corner case that might affect performance.
This is not a functional issue. In the worst case it generates both the runtime check and computes the slightly more expensive runtime trip-count in the preheader.



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/sve-tail-folding-overflow-checks.ll:45
+;
+entry:
+  %cmp6.not = icmp eq i32 %N, 0
----------------
david-arm wrote:
> nit: Maybe the entry blocks in both tests can be removed to simplify the IR and CHECK lines? I think the only thing that matters here is the `zext` in the `for.body.preheader`, right?
When I change that, ScalarEvolution doesn't recognise the maximum number of iterations that way, and so it vectorizes with the different tail folding style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142894