Page MenuHomePhabricator

[AVR] Set R31R30 as clobbered after ADJCALLSTACKUP
Needs ReviewPublic

Authored by aykevl on Mar 1 2021, 4:30 PM.

Details

Summary

In most cases, using R31R30 is fine because the call (which always
precedes ADJCALLSTACKUP) will clobber R31R30 anyway. However, in some
rare cases the register allocator might insert an instruction between
the call and the ADJCALLSTACKUP instruction and expect the register
pair to be live afterwards. I think this happens as a result of
rematerialization. Therefore, to fix this, the instruction needs to have
Defs set to R31R30.

Setting the Defs field does have the effect of making the instruction
look dead, which it certainly is not. This is fixed by setting
hasSideEffects to true.


Note that I can only reproduce the bug on a special branch that I'm working on. Maybe it is reproduce outside that branch with some effort. However, because it looks like a potential bug (and because it fixes a TODO) I've decided to create a patch to fix this anyway.

Diff Detail

Event Timeline

aykevl created this revision.Mar 1 2021, 4:30 PM
aykevl requested review of this revision.Mar 1 2021, 4:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 4:30 PM
aykevl planned changes to this revision.Mar 2 2021, 6:18 AM

I think there is a better way to do this: by always using a frame pointer when a function needs a call frame. That would simplify the code a lot.

aykevl requested review of this revision.EditedMar 2 2021, 4:08 PM
aykevl retitled this revision from [AVR] Set R31R30 as clobbered after ADJCALLSTACKDOWN to [AVR] Set R31R30 as clobbered after ADJCALLSTACKUP.
aykevl edited the summary of this revision. (Show Details)

Nevermind, that won't work. This looks like the right way forward.

I have two other patches that fix a similar bug in ADJCALLSTACKDOWN. That fix is unfortunately a lot harder.