[PATCH] D92934: [ARM][MachineOutliner] Add stack fixup feature.

Mon Jan 18 05:30:31 PST 2021

yroux added a comment.

Hi Sam,

no worries ;)

================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5899
+
+  switch (AddrMode) {
+  case ARMII::AddrMode3:
----------------
samparker wrote:
> How about extracting this out into ARMBaseInstrInfo? There seems to be a fair number of places in the backend where getting the NumBits and Scale comes up.
Yes I was thinking of that kind of refactoring and I'll look at it, now if you are ok with that peace of code, what about committing it as it is such that we can have full version of the machine outliner in LLVM-12.  And I'll propose a refactoring patch ASAP (since will touch BaseRegisterInfo, ConstantIsland, etc... I guess it will take some time to validate it properly)

================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:6375
+  // Walk over the basic block and fix up all the stack accesses.
+  fixupPostOutline(MBB);
 }
----------------
samparker wrote:
> Does this mean that we can call fixupPostOutline twice on MBB? I'm confused why we're checking for saving LR here, when it looks like we'd know for sure around line 6351.
we have 2 cases where LR is saved on the stack and we need to fixup its access:
* when we have a call inside the outlined region, we are saving/restoring LR at the begining/end of this block, it is the case handled at line 6351
* but also, at call site when LR is read after the outlined region of one of the candidates for instance (this is the MachineOutlinerDefault mode) we have to fixup the stack access as well (this case)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92934/new/

https://reviews.llvm.org/D92934