Page MenuHomePhabricator

[ARM] Fix PR35481
ClosedPublic

Authored by chill on Dec 7 2017, 8:34 AM.

Details

Summary

The issue is when restoring LR when there are no available low registers after popping the callee-saves and we try to find a temporary register before the pop. In the testcase, we are in front of a pop {r7}, however r7 is not in the pop-friendly set (since it's used as a frame pointer), whereas the code assumes that at least one register in the pop is in that set.

This patch allows r7 to be used, regardless of its use as a frame pointer, and also just falls back to using a high register if, for some reason,
we weren't able to find a suitable temporary reg among the popped ones.

Fixes https://bugs.llvm.org/show_bug.cgi?id=35481

Diff Detail

Repository
rL LLVM

Event Timeline

chill created this revision.Dec 7 2017, 8:34 AM
efriedma added inline comments.Jan 2 2018, 12:09 PM
lib/Target/ARM/Thumb1FrameLowering.cpp
618 ↗(On Diff #125969)

If you want exactly the set r0...r7, shouldn't you just set that, rather than getting the set of allocatable registers and adding back specific registers? That would simplify the code a bit because findTemporariesForLR would be guaranteed to succeed.

chill added inline comments.Jan 3 2018, 2:55 AM
lib/Target/ARM/Thumb1FrameLowering.cpp
618 ↗(On Diff #125969)

I'd prefer to not depend (or depends less) on which registers are considered allocatable and contain this decision in ARMBaseRegisterInfo::getReservedRegs - to deal with platform specific conventions, possible future options like GCC's -ffixed-reg-N, etc.
That said, I'd like to add back in a similar manner r6/BasePtr.

chill marked an inline comment as done.Jan 3 2018, 8:01 AM
chill added inline comments.
lib/Target/ARM/Thumb1FrameLowering.cpp
618 ↗(On Diff #125969)

That said, I'd like to add back in a similar manner r6/BasePtr.

I wasn't able to produce a testcase which would take advantage of having a reserved r6 in that set, so I'll
leave it as is.

efriedma accepted this revision.Jan 3 2018, 2:07 PM

LGTM.

lib/Target/ARM/Thumb1FrameLowering.cpp
618 ↗(On Diff #125969)

Okay.

This revision is now accepted and ready to land.Jan 3 2018, 2:07 PM
This revision was automatically updated to reflect the committed changes.
chill marked an inline comment as done.