[LV][NFC-ish] Allow vector widths over 256 elements

[simoll]

Summary

The assertion that vector widths are <= 256 elements was hard wired in the LV code. Eg, VE allows for vectors up to 512 elements. Test again the TTI vector register bit width instead - this is an NFC for non-asserting builds.

[kaz7]

I think removing this is good idea, but I'm not sure why the maximum vector size was limited to 64 and recently jumped up to 256. So, I cannot say LGTM atm. Does anyone know background on this?

[simoll]

Here is that prior commit that changed this - i guess, we can just commit our change, if this looks good to you.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

5352

Could we just use TTI->getRegisterBitWidth as upper bound? That would allow us to keep the assert as a sanity check.

[simoll]

Followed @fhahn 's suggestion to test against the widest vector register width instead.

[kaz7]

I thought it's a good idea when I hear it from @fhahn, but... I think It's not a good idea since 1) WidestRegister holds bit width, 2) MaxVectorSize is calculated from TTI->getRegisterBitWidth anyway.

[fhahn]

In D91518#2404870, @kaz7 wrote:

I thought it's a good idea when I hear it from @fhahn, but... I think It's not a good idea since 1) WidestRegister holds bit width, 2) MaxVectorSize is calculated from TTI->getRegisterBitWidth anyway.

I think it somewhat preserves the spirit of the assertion, which IIUC was added to ensure none of the calculations above go rouge on lead to huge vectorization factors. The way the computation is supposed to work, dividing the widest register in bits by the smallest possible type width in bits (1) seems a suitable upper bound to preserve the spirit of the assert (I'd say it's debatable whether the assert itself adds a lot of protection, but it does at least add a little bit).

This LGTM, but happy to discuss this further.

[kaz7]

In D91518#2404924, @fhahn wrote:

dividing the widest register in bits by the smallest possible type width in bits (1) seems a suitable upper bound to preserve the spirit of the assert

That's make sense. I understand now. Thank you for explanations.

[simoll]

The current versions seems a good middle ground between computing a tighter bound and abolishing the assertion altogether. Trying to be smart in assertions is usually a bad idea because people might pull the carpet (underlying assumptions) from under you some day.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

LoopVectorizationCostModel::computeFeasibleMaxVF(unsigned ConstTripCount) {
   // Note that both WidestRegister and WidestType may not be a powers of 2.
   unsigned MaxVectorSize = PowerOf2Floor(WidestRegister / WidestType);
 
   LLVM_DEBUG(dbgs() << "LV: The Smallest and Widest types: " << SmallestType
                     << " / " << WidestType << " bits.\n");
   LLVM_DEBUG(dbgs() << "LV: The Widest register safe to use is: "
                     << WidestRegister << " bits.\n");
 
-  assert(MaxVectorSize <= 256 && "Did not expect to pack so many elements"
-                                 " into one vector!");
+  assert(MaxVectorSize <= WidestRegister &&
+         "Did not expect to pack so many elements"
   if (MaxVectorSize == 0) {
     LLVM_DEBUG(dbgs() << "LV: The target has no vector registers.\n");
     MaxVectorSize = 1;
     return ElementCount::getFixed(MaxVectorSize);
   } else if (ConstTripCount && ConstTripCount < MaxVectorSize &&
              isPowerOf2_32(ConstTripCount)) {
     // We need to clamp the VF to be the ConstTripCount. There is no point in
     // choosing a higher viable VF as done in the loop below.