[PATCH] D76069: [ARM][MachineOutliner] Add default mode.

Thu Jul 23 05:30:34 PDT 2020

yroux added inline comments.

================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:6083
+  if (!MBB.isLiveIn(ARM::LR))
+    MBB.addLiveIn(ARM::LR);
   // Can we save to a register?
----------------
efriedma wrote:
> In general, calling addLiveIn(LR) on the current block isn't sufficient to update the liveness correctly.  If LR is live-in in the current block, it has to be live-out of that block's predecessors, which means it has to be live-in to the current block's predecessors.  See D78586.
> 
> Actually, can the isLiveIn() check fail?  if LR isn't live-in, wouldn't we choose MachineOutlinerNoLRSave?  Or is it possible to hit some weird case where LR is live, but not explicitly listed in the live-in list?
Right, when a block ends with a BX_RET for instance, LR is not marked as live-in and if we outline a chunk before  LR as to be saved and restored in a register or the stack, I'm pretty sure I had to add LR to the live-in set because the Machine Verifier wasn't happy of this explicit use, but I don't  manage to reproduce that behavior...

I'll remove the check If that's not needed anymore  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76069