[PATCH] D28782: [AMDGPU] Do not allow register coalescer to create big superregs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:1484-1486
+  unsigned SrcSize = SrcRC->getSize();
+  unsigned DstSize = DstRC->getSize();
+  unsigned NewSize = NewRC->getSize();
----------------
arsenm wrote:
> rampitec wrote:
> > arsenm wrote:
> > > This isn't being used for the spill size, so this is supposed to use getRegBitWidth
> > What do you mean?
> > 
> > 
> > ```
> > /// getSize - Return the size of the register in bytes, which is also the size
> > /// of a stack slot allocated to hold a spilled copy of this register.
> > ```
> Since https://reviews.llvm.org/D24631 the TargetRegisterClass is supposed to be considered only the spill size, which may be different from the register bit width
It is not yet committed, and AMDGPU::getRegBitWidth() is not even close to handle all register classes. When it is submitted it can be reconsidered, but I guess it should auto generate defaults to prevent "Unexpected register class" assertion.


================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:1491-1493
+  // Always allow dword and sub-dword coalescing.
+  if (SrcSize <= 4 || DstSize <= 4)
+    return true;
----------------
arsenm wrote:
> rampitec wrote:
> > arsenm wrote:
> > > We don't have sub-dword registers, so the < and comment are misleading
> > This is for packed f16, we do not want to revisit this.
> Even with packed f16 we don't have smaller sub registers than 4
I'm not sure it will stay this way in the future. Anyway, comment is updated, but I do not want to remove "<".


Repository:
  rL LLVM

https://reviews.llvm.org/D28782