[PATCH] D57254: [Outliner] Set nounwind for outlined functions

[Yvan Roux via Phabricator via llvm-commits]

yroux added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineOutliner.cpp:1134
+      }))
+    F->addFnAttr(Attribute::NoUnwind);
+
----------------
dmgreen wrote:
> efriedma wrote:
> > dmgreen wrote:
> > > efriedma wrote:
> > > > The outliner will refuse to outline calls that might unwind; you don't need the hasFnAttribute check.  (It might be possible in theory, but it would be complicated to construct the correct unwind info.)
> > > That would certainly make this simpler.
> > > 
> > > I don't see any code that would stop calls that may throw from being outlined, though. And some examples seem to show it is happen at the moment (looking at aarch64, if I'm getting this right).
> > > 
> > > Do you think it _should_ be disabled for unwindable calls? It seems to work at the moment for the simple examples I attempted.
> > We should never outline calls unless we can examine the callee to prove it doesn't depend on the caller's stack layout (which also implies it can't unwind).  See https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp#L5305 .
> > 
> > Looking at the code more carefully, it's possible it doesn't handle tail calls correctly; not sure.
> From what I can tell, buildOutlinedFrame only needs to make stack adjustments if the function is not a tailcall, or won't become a tail call through MachineOutlinerThunk. Are there other reasons I am not thinking of that would mean the stack layout is otherwise altered?
> 
> Perhps put another way, what might be the problem of throwing through a tailcall or thunk, so long as it had some cfi/exidx records?
Yes, since we are not modifying the stack for tailcall or thunk I don't see any issue with these cases


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

https://reviews.llvm.org/D57254