Page MenuHomePhabricator

[MachineOutliner] Ensure AArch64 outliner doesn't mess with any register that overlaps with LR
ClosedPublic

Authored by paquette on Aug 7 2017, 4:28 PM.

Details

Summary

It's currently possible for the AArch64 outliner to mark instructions that use w30 as legal.

This wasn't exposed in any tests until I started improving candidate selection recently. Since w30 overlaps with LR, it shouldn't be possible to outline anything that uses it.

I've also attached a MIR test that handles this. Currently, the outliner will incorrectly pull out the STRHHroW %w16, %x9, %w30, 1, 1 instructions despite them using w30.

The existing MachineOutliner IR tests should be ported over to MIR in a separate patch.

Diff Detail

Event Timeline

paquette created this revision.Aug 7 2017, 4:28 PM
MatzeB accepted this revision.Aug 7 2017, 4:32 PM

This looks simple enough and comes with a test so LGTM.
So I'd suggest making an attempt to stay with readsReg()/modifiesRegister() before comitting.

lib/Target/AArch64/AArch64InstrInfo.cpp
4509

modifiesRegister()/readsRegister() can figure this out for you as well if you pass in a TRI argument (without that it has no idea about aliases unfortunately).

This revision is now accepted and ready to land.Aug 7 2017, 4:32 PM
paquette marked an inline comment as done.Aug 8 2017, 2:44 PM
paquette added inline comments.
lib/Target/AArch64/AArch64InstrInfo.cpp
4509

Oh neat, that's much better.

It requires that I use W30 rather than LR, which is a little less intuitive, but I like it more.

paquette updated this revision to Diff 110285.Aug 8 2017, 2:45 PM
paquette marked an inline comment as done.

Updated the diff to reflect taking Matthias' advice.

I'm going to go ahead and commit this now.

MatzeB closed this revision.Sep 26 2017, 2:17 PM

Looks like this was committed in r310422