This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Minor SReg64 register class refactoring
AbandonedPublic

Authored by kzhuravl on Apr 9 2017, 11:14 PM.

Details

Reviewers
arsenm
Summary
  • Introduce register class for SReg_64 with subregisters
  • Introduce register class for aperture registers
  • Make aperture register class a part of SReg_64 register class

Diff Detail

Event Timeline

kzhuravl created this revision.Apr 9 2017, 11:14 PM
arsenm added inline comments.Apr 10 2017, 10:52 AM
lib/Target/AMDGPU/SIRegisterInfo.td
133

Aren't these 32bit only, so i32?

306

It would be less annoying to keep this with the same name and to rename the other one

313

This is adding 32-bit registers to the 64-bit register class?

315

The set with the aperture regs should be unallocatable

arsenm edited edge metadata.Apr 10 2017, 10:55 AM

I think this requires changing the register class for SSrc_32/VSrc_32 to be the one that includes the special 32-bit regs. I think I started doing this to start supporting vccz and the other special 1-bit inputs

I think this requires changing the register class for SSrc_32/VSrc_32 to be the one that includes the special 32-bit regs. I think I started doing this to start supporting vccz and the other special 1-bit inputs

I do not think so. src_*_base and src_*_limit are 64 bit inline constants. Here is an example of src_shared_base:

{SMB.shared_base[15:0], 48’h000000000000}

src_*_base and src_*_limit also do not have subregisters.

lib/Target/AMDGPU/SIRegisterInfo.td
133

Aperture registers are 64 bit inline constants.

306

Which one?

313

Those are 64 bit inline constants, hence in 64 bit register class.

315

APERTURE_REGS is let isAllocatable = 0.

I think this requires changing the register class for SSrc_32/VSrc_32 to be the one that includes the special 32-bit regs. I think I started doing this to start supporting vccz and the other special 1-bit inputs

I do not think so. src_*_base and src_*_limit are 64 bit inline constants. Here is an example of src_shared_base:

{SMB.shared_base[15:0], 48’h000000000000}

src_*_base and src_*_limit also do not have subregisters.

According to the manual it says if used with a 32-bit input, the 32 LSBs are used, so I guess it has sub0 but not sub1

arsenm added inline comments.Apr 10 2017, 3:28 PM
lib/Target/AMDGPU/SIRegisterInfo.td
306

I mean SReg_64 remains as it is and add SReg_64_WithApertureRegs or something

315

The class containing it also needs to be unallocatable (so having the allocation priority set also doesn't make sense)

While you're working on this, can you implement TargetRegisterInfo::isConstantPhysReg for these?

While you're working on this, can you implement TargetRegisterInfo::isConstantPhysReg for these?

Sure.

kzhuravl planned changes to this revision.Apr 17 2017, 11:20 AM
kzhuravl abandoned this revision.Apr 2 2019, 12:43 PM