[PATCH] D147468: [VPlan] Add VFRange::begin() and end() iterators. (NFCI)

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8052
 
-  for (ElementCount TmpVF = Range.Start * 2;
-       ElementCount::isKnownLT(TmpVF, Range.End); TmpVF *= 2)
+  for (ElementCount TmpVF : VFRange(Range.Start * 2, Range.End))
     if (Predicate(TmpVF) != PredicateAtRangeStart) {
----------------
Ayal wrote:
> nit (Independent of this patch): can iterate over entire Range, rather than excluding its start element.
Excluding the start seems to be a minor optimization. I can remove it, but not sure if it improves things by much?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8898
   bool IVUpdateMayOverflow = false;
-  for (ElementCount VF = Range.Start;
-       ElementCount::isKnownLT(VF, Range.End); VF *= 2)
+  for (ElementCount VF : Range) {
     IVUpdateMayOverflow |= !isIndvarOverflowCheckKnownFalse(&CM, VF);
----------------
Ayal wrote:
> nit (here and below): are the parentheses needed?
Remove all these in the committed version, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:125
+    ElementCount EndVF = End;
+    // Make sure the end iterator is a power-of-2 so != comparisons with end work as expected.
+    if (!isPowerOf2_64(End.getKnownMinValue()))
----------------
Ayal wrote:
> TODO: instead of implicitly bumping End to next power-of-2 here, set it to MaxVFTimesTwo originally instead of MaxVFPlusOne and assert both Start and End are powers-of-two upon construction above?
This should be done in c7a34d355a61396d438ea095e1e6996cde1ef880, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147468