Page MenuHomePhabricator

[AMDGPU] Fix SGPR fixing through SCC chaining
ClosedPublic

Authored by hliao on Mar 14 2019, 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.Mar 14 2019, 6:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 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.Mar 14 2019, 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.Mar 14 2019, 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.Mar 14 2019, 9:36 AM

Add minimal MIR test

hliao marked 2 inline comments as done.Mar 14 2019, 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.Mar 14 2019, 10:03 AM
arsenm accepted this revision.Mar 14 2019, 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.Mar 14 2019, 6:58 PM
This revision was automatically updated to reflect the committed changes.