[AMDGPU] Stop using getMinimalPhysRegClass in LowerFormalArguments

[foad]

Summary

NFCI. The motivation for this is avoid problems in future if we add new
classes containing only a subset of all VGPRs, or a subset of all SGPRs.
getMinimalPhysRegClass would favour these smaller classes, which is not
what we want here.

[foad]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

2551

As an alternative, I did try implementing a getMaximalAllocatablePhysRegClass and using it here. That seemed to cause other problems, but I can pursue it further if you think it's the right way to go.

[Joe_Nash]

I think a switch case over known RegClasses that we want to use is the way to go. The only question is do we need to handle more cases. Is this passing testing? When I put assert(VT.getSizeInBits() == 32) here I get massive test failures. So what happens to wider types, they all become VGPR_32?

[arsenm]

In D121914#3389338, @Joe_Nash wrote:

I think a switch case over known RegClasses that we want to use is the way to go. The only question is do we need to handle more cases. Is this passing testing? When I put assert(VT.getSizeInBits() == 32) here I get massive test failures. So what happens to wider types, they all become VGPR_32?

All arguments are supposed to be split into 32 bit pieces before they reach here

[arsenm]

In D121914#3389343, @arsenm wrote:

In D121914#3389338, @Joe_Nash wrote:

I think a switch case over known RegClasses that we want to use is the way to go. The only question is do we need to handle more cases. Is this passing testing? When I put assert(VT.getSizeInBits() == 32) here I get massive test failures. So what happens to wider types, they all become VGPR_32?

All arguments are supposed to be split into 32 bit pieces before they reach here

I'm assuming your failures are on 16-bit types which still use a 32-bit register class

[foad]

In D121914#3389338, @Joe_Nash wrote:

Is this passing testing?

Yes.

When I put assert(VT.getSizeInBits() == 32) here I get massive test failures. So what happens to wider types, they all become VGPR_32?

See the calling conv definitions in AMDGPUCallingConv.td. All supported types (including i16 and f16) are passed in 32-bit GPRs, either VGPRn or SGPRn.

[Joe_Nash]

In D121914#3389344, @arsenm wrote:

In D121914#3389343, @arsenm wrote:

In D121914#3389338, @Joe_Nash wrote:

I think a switch case over known RegClasses that we want to use is the way to go. The only question is do we need to handle more cases. Is this passing testing? When I put assert(VT.getSizeInBits() == 32) here I get massive test failures. So what happens to wider types, they all become VGPR_32?

All arguments are supposed to be split into 32 bit pieces before they reach here

I'm assuming your failures are on 16-bit types which still use a 32-bit register class

Yep. When I put assert(VT.getSizeInBits() <= 32) they go away.

Ok. LGTM

[foad]

In D121914#3389338, @Joe_Nash wrote:

I think a switch case over known RegClasses that we want to use is the way to go.

Do you prefer this (cribbed from SITargetLowering::insertCopiesSplitCSR)?

const TargetRegisterClass *RC = nullptr;
if (AMDGPU::SGPR_32RegClass.contains(Reg))
  RC = &AMDGPU::SGPR_32RegClass;
else if (AMDGPU::VGPR_32RegClass.contains(Reg))
  RC = &AMDGPU::VGPR_32RegClass;
else
  llvm_unreachable("Unexpected register class in LowerFormalArguments!");

[Joe_Nash]

As you say, it seems guaranteed to be SGPR or VGPR by the calling convention. They compile to the same thing under optimization right? If so I slightly prefer the llvm_unreachable version because the intent is locally clear.

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

if (IsEntryFunc && VA.isMemLoc()) {
       if (!Arg.Flags.isByVal())
         Chains.push_back(Val.getValue(1));
       continue;
     }
 
     assert(VA.isRegLoc() && "Parameter must be in a register!");
 
     Register Reg = VA.getLocReg();
-    const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg, VT);
+    const TargetRegisterClass *RC = nullptr;
+    if (AMDGPU::VGPR_32RegClass.contains(Reg))
+      RC = &AMDGPU::VGPR_32RegClass;
+    else if (AMDGPU::SGPR_32RegClass.contains(Reg))
+      RC = &AMDGPU::SGPR_32RegClass;
+    else
+      llvm_unreachable("Unexpected register class in LowerFormalArguments!");
     EVT ValVT = VA.getValVT();
 
     Reg = MF.addLiveIn(Reg, RC);
     SDValue Val = DAG.getCopyFromReg(Chain, DL, Reg, VT);
 
     if (Arg.Flags.isSRet()) {
       // The return object should be reasonably addressable.