Page MenuHomePhabricator

Handle subregs and superregs in callee-saved register mask
AcceptedPublic

Authored by jrtc27 on Jan 22 2020, 3:43 AM.

Details

Summary

If a target lists both a subreg and a superreg in a callee-saved
register mask, the prolog will spill both aliasing registers. Instead,
don't spill the subreg if a superreg is being spilled. This case is hit by the
PowerPC SPE code, as well as a modified RISC-V backend for CHERI I maintain out
of tree.

Event Timeline

jrtc27 created this revision.Jan 22 2020, 3:43 AM
bjope added a subscriber: bjope.Jan 22 2020, 5:31 AM
jhibbits accepted this revision.Jan 22 2020, 7:03 AM

Thanks for doing this! It's been on my TODO list for a little while already for SPE, but I never got around to it.

Is there any provision for spilling only the subreg, not the superreg? It seems to me that whenever the subreg needs spilled, it always also spills the superreg, even if the full superreg is not used.

This revision is now accepted and ready to land.Jan 22 2020, 7:03 AM
fhahn added a subscriber: fhahn.Jan 22 2020, 11:10 AM
fhahn added inline comments.
llvm/lib/CodeGen/PrologEpilogInserter.cpp
404

nit: I think you could use MCRegisterInfo::subregs() which is slightly less code.

arsenm added a subscriber: arsenm.Jan 22 2020, 11:13 AM
arsenm added inline comments.
llvm/lib/CodeGen/PrologEpilogInserter.cpp
404

These masks should eventually be in terms of regunits

jrtc27 marked an inline comment as done.Jan 23 2020, 2:37 AM

Thanks for doing this! It's been on my TODO list for a little while already for SPE, but I never got around to it.

Is there any provision for spilling only the subreg, not the superreg? It seems to me that whenever the subreg needs spilled, it always also spills the superreg, even if the full superreg is not used.

No, because restoring a subreg often has the side-effect of clearing the rest of the superreg, thereby violating the callee-preserved nature of them. I don't know whether that applies to SPE or not (and therefore whether the previously generated code was technically wrong), but it certainly applies to CHERI RISC-V.

llvm/lib/CodeGen/PrologEpilogInserter.cpp
404

Ah yes, I remember seeing MCRegisterInfo::subregs() go in but originally wrote this patch (not for SPE) over a year ago. I will update to the nicer API.

jrtc27 updated this revision to Diff 240021.Jan 23 2020, 2:57 PM

Switch from explicit iterator to recently-added range-based for loop interface. Ok to merge?

Ping?

Ping landing this?