[PATCH] D125235: [LoopVectorize] Add overflow checks when tail-folding with scalable vectors

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2980-2982
+    // NOTE: If we restrict vscale in the future to only be a power-of-2, then
+    // we can remove this code!
+
----------------
Can you remove this?  The following paragraph pretty much implies the same and I don't see such a langref change ever happening.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2983
+
+    // We can't assume that vscale is always a power-of-2, so we have to deal
+    // with overflow. We only execute the vector loop if we know the IV will
----------------
Perhaps "VScale is not necessarily a power-of-2, which means we cannot guarantee an overflow to zero when updating induction variables and so an additional overflow check is required before entering the vector loop."?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2992
+
+    // Don't execute the vector loop if (UMax - n) < (VF * UF).
+    CheckMinIters = Builder.CreateICmp(ICmpInst::ICMP_ULT, LHS, Step);
----------------
Not sure if this is better so feel free to ignore but what about doing `n + (VF * UF) < n`? to remove the need for the mask.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125235