[PATCH] D96021: [LoopVectorize] NFC: Move UserVF feasibility checks to separate function.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 18 14:49:54 PST 2021


fhahn added a comment.

I had a look at the next patch and I am not sure if it is necessary to split out the `UserVF` handling (which in turn requires splitting out the code to limit the VF by the the maximum safe VF, D96022 <https://reviews.llvm.org/D96022>). Unfortunately the code in computeFeasibleMaxVF has not been the most straight-forward, even before adding scalable support and I am a bit worried things will get harder to follow if the code is spread out more.

I think it might be simpler to operate in 3 stages in computeFeasibleMaxVF: 1) compute maximum fixed & scalable VFs, 2) use computed max VFs to pick/limit UserVF, if provided and 3) pick MaxVF based on registers, limit by computed MaxVF. I sketched out this approach in D96997 <https://reviews.llvm.org/D96997>.

I have not looked at the other patches in this stack in detail, so I might be missing something that makes things more complicated. But to pick a scalable VF, we'd just have to split out the logic to compute the `MaxVectorSize` (something like `computeMaxVFBasedOnRegisters`) and then call it for both scalable & fixed width vectors and limit by the computed maximum VFs. I think that would be similar to what's done in D96023 <https://reviews.llvm.org/D96023>, but overall I think it might be simpler than the `clampFeasibleMaxVF` approach, because less parameters need to be passed around and we also avoid re-computing the max VFs to clamp with.


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

https://reviews.llvm.org/D96021



More information about the llvm-commits mailing list