This is an archive of the discontinued LLVM Phabricator instance.

[mips] Use register scavenging with MSA.
ClosedPublic

Authored by sdardis on Oct 18 2017, 7:12 AM.

Details

Summary

MSA stores and loads to the stack are more likely to require an
emergency GPR spill slot due to the smaller offsets available
with those instructions.

Handle this by overestimating the size of the stack by determining
the largest offset presuming that all callee save registers are
spilled and accounting of incoming arguments when determining
whether an emergency spill slot is required.

Diff Detail

Repository
rL LLVM

Event Timeline

sdardis created this revision.Oct 18 2017, 7:12 AM
atanasyan added inline comments.Oct 23 2017, 9:11 AM
lib/Target/Mips/MipsFrameLowering.cpp
119 ↗(On Diff #119482)

Was the initial expression with - sign a bug/typo?

sdardis edited the summary of this revision. (Show Details)Oct 24 2017, 8:17 AM
sdardis added inline comments.Oct 25 2017, 8:01 AM
lib/Target/Mips/MipsFrameLowering.cpp
119 ↗(On Diff #119482)

I thought this was a bug, but it's not quite.

This function was estimating the size of the stack, but doesn't account for incoming arguments. MachineFrameInfo::estimateStackSize() perfroms most of the calculations here, but doesn't account for fixed objects which represent incoming arguments or spilling all callee-saved registers.

I'll update this shortly.

sdardis updated this revision to Diff 120388.Oct 26 2017, 3:52 AM

Update the mechanism for estimating the size of the stack.

atanasyan added inline comments.Oct 26 2017, 8:49 AM
lib/Target/Mips/MipsFrameLowering.cpp
124 ↗(On Diff #120388)

So now we save to the Size a "positive" size of an object with minimal index. Is it intended? Should we sum all the sizes?

sdardis added inline comments.Oct 26 2017, 8:53 AM
lib/Target/Mips/MipsFrameLowering.cpp
124 ↗(On Diff #120388)

D'oh. Yes, that should be +=.

atanasyan accepted this revision.Oct 31 2017, 1:52 AM

LGTM with the += change.

This revision is now accepted and ready to land.Oct 31 2017, 1:52 AM
sdardis updated this revision to Diff 120950.Oct 31 2017, 2:57 AM

Update before commit.

This revision was automatically updated to reflect the committed changes.