[PATCH] D80870: [LV] Make sure smallest/widest type sizes are powers-of-2.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

> Alternatively we could force the computed max VF to the next-lowest power-of-2

This alternative may perhaps be better, or another one, as noted inline.

Also added a couple of unrelated comments.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5011
 
   unsigned MaxVF = UserVF ? UserVF : computeFeasibleMaxVF(TC);
   assert((UserVF || isPowerOf2_32(MaxVF)) && "MaxVF must be a power of 2");
----------------
The original motivation to assert that MaxVF is a power of 2, in D80491, was for concluding that any chosen VF will divide a constant trip count.
Perhaps a better fix than D80491 would be (have been ..., as noted in the summary) to round MaxVF down to a power of 2 here, rather than forcing computeFeasibleMaxVF() and/or getSmallestAndWidestTypes() to return powers of 2.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5078
   } else if (ConstTripCount && ConstTripCount < MaxVectorSize &&
              isPowerOf2_32(ConstTripCount)) {
     // We need to clamp the VF to be the ConstTripCount. There is no point in
----------------
Unrelated: makes sense to limit MaxVF to ConstTripCount also if the latter is not a power of 2, by rounding it down to a power of 2 (or up, with fold-tail?).


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5118
                           << ") with target's minimum: " << MinVF << '\n');
         MaxVF = MinVF;
       }
----------------
Unrelated: getMinimumVF() must also be (asserted to be) a power of 2; moreover, it must not cause MaxVF to exceed `MaxSafeRegisterWidth`.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5229
 
-  return {MinWidth, MaxWidth};
+  // Round up to the next power of 2, if min or max widths aren't powers of 2.
+  return {PowerOf2Ceil(MinWidth), PowerOf2Ceil(MaxWidth)};
----------------
The critical part is to make sure VF does not exceed `MaxSafeRegisterWidth`; this indeed holds when rounding `MinWidth` and `MaxWidth` up to powers of 2; might not hold when rounding them down, conceptually.

But perhaps non-power-of-2 sized types should be skipped when computing MinWidth and MaxWidth, related to the last "also" part of the FIXME above? cf. `hasIrregularType()`. 


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/fp80-widest-type.ll:9
+
+define x86_fp80 @test() {
+; CHECK-LABEL: @test(
----------------
Worth indicating that this fixes PR46139; e.g., in a comment, test name, file name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80870