Page MenuHomePhabricator

[AMDGPU] Fix SGPR fixing through SCC chaining
ClosedPublic

Authored by hliao on Thu, Mar 14, 6:37 AM.

Details

Summary
  • During the fixing of SGPR copying from VGPR, ensure users of SCC is properly propagated, i.e.
    • only propagate through live def of SCC,
    • skip the SCC-def inst itself, and
    • stop the propagation on the other SCC-def inst after checking its SCC-use first.

Diff Detail

Repository
rL LLVM

Event Timeline

hliao created this revision.Thu, Mar 14, 6:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Mar 14, 6:37 AM

I'm not sure I understand the exact problem being solved here. What are the observable consequences?

llvm/test/CodeGen/AMDGPU/udivrem64.ll
172–184 ↗(On Diff #190611)

I don't see how this is stressing this. Can you add another, more targeted test for this? The div expansion is huge

hliao marked an inline comment as done.Thu, Mar 14, 7:55 AM
hliao added inline comments.
llvm/test/CodeGen/AMDGPU/udivrem64.ll
172–184 ↗(On Diff #190611)

This is case where I found the long SCC chain(s). Yeah, the expansion is huge. But, to constructing a SCC chain with parts of instructions without SGPR dependency but SCC dependency is hard, also that sequence needs part of code has to use VGPR. Could you just use this minimalist LLVM IR test?

arsenm added inline comments.Thu, Mar 14, 8:08 AM
llvm/test/CodeGen/AMDGPU/udivrem64.ll
172–184 ↗(On Diff #190611)

You can also use a MIR test

hliao updated this revision to Diff 190652.Thu, Mar 14, 9:36 AM

Add minimal MIR test

hliao marked 2 inline comments as done.Thu, Mar 14, 9:37 AM
hliao added inline comments.
llvm/test/CodeGen/AMDGPU/udivrem64.ll
172–184 ↗(On Diff #190611)

added in the latest patch

hliao edited reviewers, added: rampitec; removed: stanblesk.Thu, Mar 14, 10:03 AM
arsenm accepted this revision.Thu, Mar 14, 6:58 PM

LGTM. I'm a bit iffy about relying on dead flags though, but I think it's OK.

This revision is now accepted and ready to land.Thu, Mar 14, 6:58 PM
This revision was automatically updated to reflect the committed changes.