efriedma added inline comments.

================
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;
----------------
yroux wrote:
> efriedma wrote:
> > 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.)
> I agree that there is no issue with SP until we modify it, I'll move that to 
> the right patch, for LR it is needed here, when doing thunk outlining the 
> call to the outlined function is a BL and if an instruction uses LR in the 
> outlined region we will have a problem.
Oh, right, we're effectively hoisting the definition of LR in thunk outlining.  
Makes sense.

For tail-call outlining, we don't care about LR, but I guess it's trickier to 
separate out those two cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76066



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to