[PATCH] D103348: [AMDGPU] Add maximum NSA size limit ISA feature

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 2 08:20:30 PDT 2021


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4125
+  if (NumAddrRegs > 4 && !isPowerOf2_32(NumAddrRegs)) {
+    // Round up elements to power of 2
+    const int RoundedNumRegs = NextPowerOf2(NumAddrRegs);
----------------
critson wrote:
> arsenm wrote:
> > Why does this need to round up? We should be able to directly handle non powers of 2
> I think the point after which this needs to round up should be higher, i.e. 6 instead of 4.
> The problem I am hitting is that it seems instruction definitions for the appropriate sizes are missing at the moment in a lot of cases.
> Perhaps the MIMG definitions predate when we added VReg_160 / VReg_192?
> I guess I can try and fix those first.
>  
> However I think we will have to round up for larger vector sizes as we do not have arbitrary register sizes beyond VReg_192?
> There are plenty of instructions which take arbitrary sizes beyond 6 VGPRs, so can  only be defined with a single VReg_256/VReg_512 argument.
We should add the register classes eventually. If we are going to workaround the missing classes, this isn't the place for it. The selector can fixup the classes when we actually need to pick a class


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103348



More information about the llvm-commits mailing list