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

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 23 05:37:20 PDT 2021


foad 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
----------------
arsenm wrote:
> 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
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104622



More information about the llvm-commits mailing list