[PATCH] D104622: [AMDGPU] Add 224-bit vector types and link 192-bit types to MVTs

Wed Jun 23 05:33:43 PDT 2021

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.td:739-746
+  // This occurs because SGPR_160 and SReg_160 classes are equivalent in size
+  // meaning their enumeration order is dependent on alphanumeric ordering of
+  // their names.  The superclass for inherence is the last one in topological
+  // order (i.e. enumeration order), hence SReg_160 is selected.
+  // Potential workarounds involve renaming SGPR_160, adding another class
+  // which is ordered last and hence used for inheritance, or adding more
+  // registers to SReg_160 to cause it to be moved earlier in the superclass
----------------
foad wrote:
> arsenm wrote:
> > foad wrote:
> > > 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?
> > No, because the TTMP_* registers also form the large tuples
> 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?
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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104622