[AMDGPU] Fix unreachable reg bit width

[Shoreshen]

Add register class bit width for SReg_256_XNULL and SReg_128_XNULL

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: None (Shoreshen)

Changes

Add register class bit width for SReg_256_XNULL and SReg_128_XNULL

---

Full diff: /~https://github.com/llvm/llvm-project/pull/122107.diff

1 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp (+2)

diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 319ada3b27bd5a..d9c0aa300855fc 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -2487,6 +2487,7 @@ unsigned getRegBitWidth(unsigned RCID) {
   case AMDGPU::AReg_128_Align2RegClassID:
   case AMDGPU::AV_128RegClassID:
   case AMDGPU::AV_128_Align2RegClassID:
+  case AMDGPU::SReg_128_XNULLRegClassID:
     return 128;
   case AMDGPU::SGPR_160RegClassID:
   case AMDGPU::SReg_160RegClassID:
@@ -2523,6 +2524,7 @@ unsigned getRegBitWidth(unsigned RCID) {
   case AMDGPU::AReg_256_Align2RegClassID:
   case AMDGPU::AV_256RegClassID:
   case AMDGPU::AV_256_Align2RegClassID:
+  case AMDGPU::SReg_256_XNULLRegClassID:
     return 256;
   case AMDGPU::SGPR_288RegClassID:
   case AMDGPU::SReg_288RegClassID:

[arsenm]

Tests?

[Shoreshen]

Tests?

Hi there is another PR depending on this so I created the PR first. Will add tests latter (need to minimize)

[arsenm]

Also the 128 case.

[Shoreshen]

Hi @arsenm , to trigger the unreachable during postmisched pass it has to be MIMG instruction, but I cannot find MIMG instruction uses SReg_128_XNULL

I also tried to find other places that may use the bit width function:

1. used in selectCOPY
2. used in foldOperand, but for immediate only
3. used in canInsertSelect test if it is ok to insert a select instruction
4. used in buildSpillLoadStore but it seems like the target registers are all caller/callee saved regs
5. used in printRegularOperand but only for Op.isDFPImm()
6. used in getRegOperandSize, which is used in validateMIMGAddrSize and validateMIMGDataSize

The only possibility is getRegSplitParts, but it is used by many function, I need help on this since I do not familiar with the fucntion.

[arsenm]

I would expect the selectCOPY case would be most straightforward

[Shoreshen]

Hi @arsenm , to successfully trigger bit width function in selectCOPY, the dst operand must:

1. pass isVCC function for the dst register of COPY
2. fails isVCC function for src register of the COPY

I think the bit width of src and dst must be the same, otherwise the copy mismatch type verification error will trigger

Thus to use SReg_128_XNULL as src reg class, the dst must also be 128 bit width

To pass the isVCC function for dst register, it must be:

1. Non physical register (which I cannot simply use $vcc as dst)
2. If it is assigned register class, the bit width must be 1
3. If it is register bank, the bank id must equals to AMDGPU::VCCRegBankID

For COPY not being assigned for reg class, I think the dst of COPY must not be used for any instruction.

Because each instruction's each input should have a reg class bind with it, and ti will try to assign reg class accordingly.

But if COPY is not used by any instruction, then it will not be selected, since it most probably will not pass !isTriviallyDead(MI, MRI) function check.

So it seems like I ran out of my ways to produce a 128 case, would there be any other possibilities?

Thanks a lot!

---

BTW, I also tried to remove the AMDGPU::getRegBitWidth(SrcRC->getID()) == 16 in selectCOPY and tried to find any test failed after then. but it seems even without the judgement, all tests passed.

[jwanggit86]

The original problem was triggered by SReg_256_XNULL. So maybe we just fix it for that reg class?

[arsenm]

Can do this in one run-pass with , separated pass names. but I'm not sure why you need to run 2 passes, or how either ne stresses this function

[arsenm]

unrelated change

[arsenm]

Rename test file

[github-actions]

@Shoreshen Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!