[PATCH] D78772: [AMDGPU] Adapt GCNRegBankReassign for 16 bit subregs

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp:456-457
+  //       parent VGPR_32 and potentially a sibling 16 bit sub-register.
+  if (Size < 32)
+    return false;
+
----------------
This kills the code below


================
Comment at: llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp:463
+  if (Size == 16)
+    PhysReg = TRI->get32BitRegister(PhysReg);
+  else if (Size > 32)
----------------
Can't you just check SGPR16_LO.contains instead of going through the size check? I would expect you can get rid of the explicit size check by just trying getSubReg/getSuperReg and see if either failed


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1789-1790
+  const TargetRegisterClass *RC = getPhysRegClass(Reg);
+  if (getRegSizeInBits(*RC) != 16)
+    return Reg;
+
----------------
Seems like this should just be an assert


================
Comment at: llvm/test/CodeGen/AMDGPU/regbank-reassign.mir:374-378
+registers:
+  - { id: 0, class: vgpr_32, preferred-register: '$vgpr1' }
+  - { id: 1, class: vgpr_32, preferred-register: '$vgpr5' }
+  - { id: 2, class: vgpr_32 }
+  - { id: 3, class: vgpr_lo16 }
----------------
If you don't need the preferred-register hints you can eliminate the registers section


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

https://reviews.llvm.org/D78772