This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Avoid constant bus limitation on V_BFE GISel pattern
ClosedPublic

Authored by Pierre-vh on Mar 15 2023, 5:28 AM.

Details

Summary

For D141247 - if that pattern was used by GISel it could cause constant bus limitation failures.
Just use inline immediates instead of S_MOV to avoid the issue.

Diff Detail

Event Timeline

Pierre-vh created this revision.Mar 15 2023, 5:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 5:28 AM
Pierre-vh requested review of this revision.Mar 15 2023, 5:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 5:28 AM
arsenm added inline comments.Mar 15 2023, 5:30 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-zext.mir
170 ↗(On Diff #505450)

Can you also add and end to end test that shows the same issue

Pierre-vh marked an inline comment as done.Mar 15 2023, 5:53 AM
Pierre-vh added inline comments.
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-zext.mir
170 ↗(On Diff #505450)

I think those get folded out later because out of all the tests affected by the constant bus limit assert, none had any regression other than the MIR tests (because they just run ISel)

[build] Failed Tests (6):
[build]   LLVM :: CodeGen/AMDGPU/GlobalISel/bswap.ll
[build]   LLVM :: CodeGen/AMDGPU/GlobalISel/cvt_f32_ubyte.ll
[build]   LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-sext.mir
[build]   LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-zext.mir
[build]   LLVM :: CodeGen/AMDGPU/GlobalISel/mul.ll
[build]   LLVM :: CodeGen/AMDGPU/mad-mix-hi.ll

I still added a small test case to show it's indeed folded out, but that one wasn't affected by the constant bus violation so not sure if it's that useful

Pierre-vh updated this revision to Diff 505453.Mar 15 2023, 5:54 AM
Pierre-vh marked an inline comment as done.

Move test to GISel folder

arsenm added inline comments.Mar 15 2023, 5:55 AM
llvm/lib/Target/AMDGPU/VOP3Instructions.td
266

These are both inline immediate so you could just directly put the immediate in the output operands

Pierre-vh updated this revision to Diff 505455.Mar 15 2023, 5:59 AM

Use inline imm

Pierre-vh edited the summary of this revision. (Show Details)Mar 15 2023, 5:59 AM
arsenm accepted this revision.Mar 15 2023, 6:40 AM
This revision is now accepted and ready to land.Mar 15 2023, 6:40 AM