This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Update SCC defs to VCC when uses are changed to VCC
ClosedPublic

Authored by bcahoon on May 8 2021, 10:50 AM.

Details

Summary

The FixSGPRCopies pass converts instructions to VALU when
removing illegal VGPR to SGPR copies. Instructions that use SCC
are changed to use VCC instead. When that happens, the pass must
also change instructions that define SCC to VCC.

The pass was not changing the SCC definition when an ADDC is
converted due to a input that is a VGPR to SGPR copy. But, the
initial ADD instruction, which define SCC, is not converted.
This causes a compilation failure due to a use of an undefined
physical register.

This patch adds code that inserts the SCC definition in the
MoveToVALU worklist when a SCC use is converted to a VCC use.

Diff Detail

Event Timeline

bcahoon created this revision.May 8 2021, 10:50 AM
bcahoon requested review of this revision.May 8 2021, 10:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2021, 10:50 AM

Can you add an IR example where this happens? I thought this was a solved problem already

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
6832–6834

modifiesRegister(VCC)

bcahoon updated this revision to Diff 344643.May 11 2021, 8:33 PM

Use modifyRegister. Added a .ll test case.

I've got a case where we see this issue as well.

Confirmed that the patch fixes it.

arsenm accepted this revision.May 13 2021, 6:22 PM

This isn't great but I also don't think it's worth putting real effort into improving SIFixSGPRCopies at this point

llvm/test/CodeGen/AMDGPU/urem-change-scc-to-scc.ll
12 ↗(On Diff #344643)

This isn't particularly enlightening. Something in the giant urem expansion triggered this. Can you come up with some IR approximating the situation in the middle?

This revision is now accepted and ready to land.May 13 2021, 6:22 PM