[PATCH] D152832: [AMDGPU] Fix register class for a subreg in GCNRewritePartialRegUses.

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:317
   if (CoverSubreg != AMDGPU::NoSubRegister)
-    return getRegClassWithShiftedSubregs(RC, Offset, CoverSubreg, SubRegs);
+    return getRegClassWithShiftedSubregs(RC, Offset, End - Offset, CoverSubreg,
+                                         SubRegs);
----------------
barannikov88 wrote:
> vpykhtin wrote:
> > arsenm wrote:
> > > Why do you need the size? In the unknown case you still have a class from the vreg 
> > I had the following problem with that, given a test:
> > ```
> > name: test_subregs_unknown_regclass_from_instructions_sgpr_1024_to_sgpr_64
> > tracksRegLiveness: true
> > registers:
> >   - { id: 0, class: sgpr_1024 }
> > body:             |
> >   bb.0:
> >     %1:vreg_64 = COPY undef %0.sub4_sub5
> > ```
> > %0.sub4_sub5 doesn't have known regclass and getSubRegisterClass returns 
> >   sgpr_1024 : sub4_sub5 -> SReg_1_with_sub0_and_SReg_1_with_lo16_in_SGPR_LO16
> > 
> > //SReg_1_with_sub0_and_SReg_1_with_lo16_in_SGPR_LO16// has the following subclasses (in tablegen generated order):
> >    VReg_1
> >    SGPR_64
> >    CCR_SGPR_64
> >    Gfx_CCR_SGPR_64
> > 
> > If I don't check the size it would select VReg_1 which is invalid.
> > 
> > I haven't followed all the consequences why tablegen generated this artificial subclass, but I know that if a class contains registers of different size tablegen puts subclass with smaller registers first, that is why VReg_1 precedes SGPR_64.
> FWIW all registers in a register class must have the same (spill) size.
> But VReg_1 shouldn't be a sublass of any class at all. This looks like a peculiar effect of a class containing no registers on tablegen.
> 
This sounds like tablegen should be fixed instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152832