[PATCH] D109300: [AMDGPU] Make vector superclasses allocatable

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 22 10:40:08 PDT 2021


rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2432
+      Idx == AMDGPU::RegisterPressureSets::AGPR_32 ||
+      Idx == AMDGPU::RegisterPressureSets::AV_32)
     return getRegPressureLimit(&AMDGPU::VGPR_32RegClass,
----------------
cdevadas wrote:
> rampitec wrote:
> > cdevadas wrote:
> > > rampitec wrote:
> > > > I do not think there can be a separate pressure on AV. What does that mean for AV pressure if you have pressure 256 on V and 256 on A? It cannot be increased or decreased separately.
> > > The assert here https://github.com/llvm/llvm-project/blob/main/llvm/utils/TableGen/CodeGenRegisters.cpp#L2028
> > > enforces at least one PSet when we make the AV classes allocatable.
> > > 
> > > There should have been a flag to entirely avoid the psets for allocatable regclasses.
> > There is GeneratePressureSet to avoid it, but the issue is conceptual. I do not understand a strategy of tracking pressure for a combined class.
> GeneratePressureSet doesn't help to avoid the PSet entirely. As you can see, for all AV classes except AV_32, GeneratePressureSet is currently zero. The moment I reset this flag for AV_32, it asserts as I indicated earlier.
> 
I did not see that assert because I was using optimized tablegen. Shall we omit the assert if GeneratePressureSet  = 0? On practice it passes testing with GeneratePressureSet = 0 and optimized tablegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109300



More information about the llvm-commits mailing list