Page MenuHomePhabricator

PowerPC: Fix register spilling for SPE registers
Needs ReviewPublic

Authored by jhibbits on Jan 14 2019, 8:38 PM.

Details

Summary

Missed in the original commit, use the correct callee-saved register
list for spilling, instead of the standard SVR432 list. This avoids
needlessly spilling the SPE non-volatile registers when they're not used.

Diff Detail

Event Timeline

jhibbits created this revision.Jan 14 2019, 8:38 PM
nemanjai requested changes to this revision.Feb 20 2019, 4:33 AM

I think this is very close, but I really think the reloading code needs to be fixed.

lib/Target/PowerPC/PPCInstrInfo.cpp
1174

This does not look right. We are in RC == nullptr section here. Why are we checking hasSubClassEq(nullptr)?

lib/Target/PowerPC/PPCRegisterInfo.cpp
165–166

These ternary operator expressions were arguably too complex already. I don't think there's any doubt that after this they're becoming nearly unparsable by a human. Can you please refactor these to use nested if's?

This revision now requires changes to proceed.Feb 20 2019, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2019, 4:33 AM
jhibbits updated this revision to Diff 188754.Feb 28 2019, 10:30 AM
jhibbits marked 2 inline comments as done.

Address feedback. Update patch to provide full context.

nemanjai requested changes to this revision.Mar 31 2019, 5:21 AM

Can you add a test case? At least for the spills that you're adding.

This revision now requires changes to proceed.Mar 31 2019, 5:21 AM

Can you add a test case? At least for the spills that you're adding.

Is there a good example for a test case testing register spills with no determined register class? That's the only location where I added new spill handling. The real meat of this should already be handled by the existing regression tests.

jhibbits updated this revision to Diff 206986.Thu, Jun 27, 6:52 PM

Reduce the scope of the patch.

jhibbits edited the summary of this revision. (Show Details)Fri, Jun 28, 7:16 AM

I removed the extra spill changes. Those can be done later if there actually is a problem. This didn't change the tests at all. I'm not quite sure how to write a test to demonstrate that "some but not all" registers are spilled in a given case, given that the register allocator can change at any time, so can't hard-code the registers that would get spilled, and I don't think I can even hard-code the number of registers that could get spilled.