[PATCH] D147158: [AMDGPU] Do not reserve 16-bit registers

Thu Mar 30 02:34:04 PDT 2023

foad added a comment.

In D147158#4231789 <https://reviews.llvm.org/D147158#4231789>, @Joe_Nash wrote:

> In D147158#4231787 <https://reviews.llvm.org/D147158#4231787>, @rampitec wrote:
>
>> In D147158#4231755 <https://reviews.llvm.org/D147158#4231755>, @Joe_Nash wrote:
>>
>>> In D147158#4231480 <https://reviews.llvm.org/D147158#4231480>, @rampitec wrote:
>>>
>>>> I am not so sure about SGPRs. I believe it is legal to use SGPR halves.

I am not sure about this. It is "legal" in that the SGPR_LO16 is allocatable, but I don't think anything uses it - except for one handwritten MIR test in `test/CodeGen/AMDGPU/fold_16bit_imm.mir`.

>>> It appears that this patch does not change anything about the legality of allocating registers in class SGPR_LO16.
>>
>> But if we will allow SGPRs this code will be needed on older targets again.
>
> I think you are right about that. Perhaps that is a a good reason not to merge this patch unless there is some immediate benefit.

I don't like guessing what will be required in future. Even with the GFX11 True16 work in progress it is unclear to me whether 16-bit SGPRs will be used.

My motivation is to clean up and simplify SIRegisterInfo::getReservedRegs based on what is required today. It seems like a lot of stuff has crept in and I am trying to understand what it is all for.

================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:615
-    if (!AMDGPU::SGPR_LO16RegClass.contains(Low))
-      Reserved.set(Low);
-  }
----------------
I've tried to preserve the intent of this code by making SReg_LO16 unallocatable but leaving SGPR_LO16 allocatable.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147158