This is an archive of the discontinued LLVM Phabricator instance.

[Thumb1] Use callee-saved register to adjust stack pointer
ClosedPublic

Authored by keith.walker.arm on Aug 9 2023, 6:03 AM.

Details

Summary

When adjusting the Stack Pointer at the end of the function epilogue,
use a callee-saved register, rather than explicitly using R4 which may
not have been saved.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 6:03 AM
keith.walker.arm requested review of this revision.Aug 9 2023, 6:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 6:03 AM

Do you have a test case for the situation where r4 is not saved?

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
552

Please add an assertion to check that ScratchRegister has been set to something.

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
552

I'll simplify the assert in the branch which adjusts SP using FP.
The other uses of the ScratchRegister in emitPrologueEpilogueSPUpdate(() already has an assert if the ScratchRegister is used.

Amended on os the asserts to make the test now match what is being set as the scratch register.

I tried to come up with a suitable additional test which didn't involve the code actually saving R4. I do have a C reproducer but it is quite a number of "fuzzed" c code and trying to turn it into a sensible IR/MIR regression test proved problematic. Also I believe that we have sufficient tests in place to ensure that the scratch register we now used is one of the callee saved registers, thus the additional test is not really required.

No additional asserts were needed to be added as all cases where ScratchRegister is used (including in the function emitPrologueEpilogueSPUpdate()) aleady has asserts in place.

This revision is now accepted and ready to land.Aug 17 2023, 7:48 AM
This revision was landed with ongoing or failed builds.Aug 17 2023, 10:30 AM
This revision was automatically updated to reflect the committed changes.