Enables Machine Outlining support on ARM for ARM and Thumb2 modes. Only the
simplest outlining modes (tailcalls and thunks) are handled here.
This is a follow-up of ARM Machine Outliner support RFC D57054
Differential D76066
[ARM][MachineOutliner] Add Machine Outliner support for ARM yroux on Mar 12 2020, 7:53 AM. Authored by
Details Enables Machine Outlining support on ARM for ARM and Thumb2 modes. Only the This is a follow-up of ARM Machine Outliner support RFC D57054
Diff Detail
Event TimelineComment Actions Hi, I like reading your code! But I'm concerned around the lack of tests here, specifically:
Comment Actions Thanks for the review Sam, I'll update the patch with more tests (for CPSR/LR it was part of D76068 but you are right it should be done here) and take your comments into account.
Comment Actions Hi Yvan, Thanks for adding the tests, I've added a few concerns and questions.
Comment Actions Hi Sam, Thanks for your comments.
Comment Actions Here is a new update of the patch. I remove the logic to disable LowOverheadLoops pass since Eli has added the live-ins infos inside outlined functions in D78605.
Comment Actions Thanks for adding the MVE changes, but I also still don't see any DSP tests, i.e QADD, SADD16.
Comment Actions hmm... I was wrong w/r to DSP instructions Q and GE flags handling, I don't get what I did in my tests, but there is no info about CPSR in the MIR attached to these instructions. So, you had a good feeling and I can disable them, but as Eli said in his comments we don't really need to worry about APCS but only to linker veneers and I don't think a linker would clear the sticky Q bit or touch the GE ones. Do you think we can let it as it is ?
Comment Actions
Ah, yes, good point. Then I have nothing else, but definitely worth waiting to see if Eli has further comments. Comment Actions Yes I'll wait for Eli's feedback and update the other patches. Thanks for the review Sam Comment Actions LGTM with one minor comment
Comment Actions I found a last issue when doing a full bootstrap with -moutline used to build clang, there was a case of thunk outlining where the original was call a BLX LR which was broken by the outlining call BL OUTLINE_FUNCTION_X I fixed this issue by checking the use of link register before handling calls and I've added a test case. Rebased and full bootstrap in ARM and Thumb2 mode are now OK, I'll commit it tomorrow if that's fine for you. |
If you control all the instructions that execute, you don't need to worry about what the procedure call standard says. You do need to worry about linker veneers if the outlined function is in a different section, though.
So you need to worry about R12/CPSR on entry to the outlined function, but not on exit from the outlined function.