[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5607
+  // * Register R12(IP),
+  // * Condition codes (and thus the CPSR register)
+  //
----------------
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.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5806
+    if (Opc == ARM::BL || Opc == ARM::tBL || Opc == ARM::BLX ||
+        Opc == ARM::tBLXr || Opc == ARM::tBLXi)
+      UnknownCallOutlineType = outliner::InstrType::LegalTerminator;
----------------
Maybe also look for TAILJMP opcodes?


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5813
+  // Don't touch the link register
+  if (MI.readsRegister(ARM::LR, TRI) || MI.modifiesRegister(ARM::LR, TRI))
+    return outliner::InstrType::Illegal;
----------------
Why do you need to forbid outlining code that touches LR or SP? None of the new instructions you're generating read or clobber them.   (It might start mattering if you add support for additional forms of outlining, or Thumb1 support, but this patch has neither.)


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5817
+  // Be conservative with program counter.
+  if (MI.readsRegister(ARM::PC, TRI) || MI.modifiesRegister(ARM::PC, TRI))
+    return outliner::InstrType::Illegal;
----------------
samparker wrote:
> Cheers. AArch64 is nice and doesn't allow arbitrary writes to the PC, but we won't have that luxury here.
You might also want to support POP_RET.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76066