Add SReg_224, VReg_224, AReg_224, etc.
Link 224-bit types with v7i32/v7f32.
Link existing 192-bit types to newly added v3i64/v3f64/v6i32/v6f32.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM, although v3 change can have unexpected impact. Could you please run PSDB on this?
llvm/test/CodeGen/AMDGPU/sdiv64.ll | ||
---|---|---|
502 | This looks like a minor regression. Why are we still loading s0? Maybe not worth holding up the patch for though. |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | Can't we get rid of the dual SGPR_*/SReg_* classes for all sizes > 64? I thought the only difference between them was that Sreg_* added non-SGPR registers like EXEC and M0, but for the larger sizes, if there are no non-SGPR registers to add, what is the point of having two different classes? |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | No, because the TTMP_* registers also form the large tuples |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | Right but currently TTMP_160, TTMP_192 etc are not defined. Maybe they should be, and the SReg_* classes should include them, and then we wouldn't be in this ambiguous situation where two classes have the same size so tablegen picks one of them arbitrarily? |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | Oh, I think the confusion is there isn't actually a set of SGPR_* classes. That's just the name of the set of registers added to the class. It would be better if the classes were named SGPR_* and we had the SReg_* classes for the addition of special regs |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | We have things like: def SGPR_192 : RegisterClass<"AMDGPU", [untyped], 32, (add SGPR_192Regs)> { let Size = 192; let AllocationPriority = 17; } So SGPR_192 is a class and SGPR_192Regs is the set of regs. I'm not sure how consistent this is for the other sizes though. |
llvm/lib/Target/AMDGPU/SIRegisterInfo.td | ||
---|---|---|
739–746 | Oh, I was thinking of VGPRs which is for some reason different. It always annoys me how you have to use VGPR_32 but everything higher is VReg_* |
I have committed this but I am happy to add the TTMP classes as a follow up.
I was unsure whether it was appropriate to add more classes that probably are not even used?
Can't we get rid of the dual SGPR_*/SReg_* classes for all sizes > 64? I thought the only difference between them was that Sreg_* added non-SGPR registers like EXEC and M0, but for the larger sizes, if there are no non-SGPR registers to add, what is the point of having two different classes?