[Outliner] Set nounwind for outlined functions

[dmgreen]

Summary

This prevents the outlined functions from pulling in a lot of unnecessary code
in our downstream libraries/linker. Which stops outlining making codesize
worse in c++ code with no-exceptions.

It appears that we can outline from functions that throw, so I've made this
only apply if all the parent functions are nounwind.

[efriedma]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

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.)

[dmgreen]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

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.

[efriedma]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

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.

[dmgreen]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

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?

[yroux]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

Yes, since we are not modifying the stack for tailcall or thunk I don't see any issue with these cases

[efriedma]

llvm/lib/CodeGen/MachineOutliner.cpp

1146

For MachineOutlinerTailCall or MachineOutlinerThunk outlining on AArch64, unwinding will never see the outlined function, so it's not an issue even if the thunk doesn't have an EH record.

The part I wasn't quite sure about glancing at the code is what would happen if callee of the call contains a tailcall that unwinds. I guess in that case the outlined function is actually on the stack:

void h(int,int,int);
__attribute((noinline)) void g(int x, int y, int z) { h(x,y,z); }
void f() { g(1,2,3); g(1,2,3); }
void f2() { g(1,2,3); g(1,2,3); }

I have no idea if we somehow generate correct unwind info here. If we do, that's fine, and I guess we want to make sure the function isn't marked nounwind, like the current version of this patch. (I'm pretty sure we're missing SEH directives on Windows, but maybe the cfi markings are enough for other targets.)

Checking whether the parent is nounwind, rather than the call itself, is overly conservative, but I guess if the parent isn't nounwind, there isn't any great benefit to marking the outlined function nounwind anyway.

[dmgreen]

I've been trying to take another look at this. I've just rebased it here onto some new tests.

It appears that the example you gave works with -fno-omit-frame-pointer/frame-pointer=non-leaf, but gets stuck with frame-pointer=none during unwinding. I do not know enough about the required unwinding info (and am having trouble finding sources for how it works) to tell exactly what is wrong.

My gut says that it is a separate issue though, and that this patch is at least conservatively correct? Even if there are existing bugs with handing throwing outlined blocks.

[efriedma]

LGTM

The unwind info the outliner constructs is in fact buggy, but I guess that's orthogonal to this issue.

llvm/lib/CodeGen/MachineOutliner.cpp

MachineFunction *MachineOutliner::createOutlinedFunction(
   Function *F = Function::Create(FunctionType::get(Type::getVoidTy(C), false),
                                  Function::ExternalLinkage, FunctionName, M);
 
   // NOTE: If this is linkonceodr, then we can take advantage of linker deduping
   // which gives us better results when we outline from linkonceodr functions.
   F->setLinkage(GlobalValue::InternalLinkage);
   F->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
 
-  // FIXME: Set nounwind, so we don't generate eh_frame? Haven't verified it's
-  // necessary.
-
   // Set optsize/minsize, so we don't insert padding between outlined
   // functions.
   F->addFnAttr(Attribute::OptimizeForSize);
   F->addFnAttr(Attribute::MinSize);
 
   // Include target features from an arbitrary candidate for the outlined
   // function. This makes sure the outlined function knows what kinds of
   // instructions are going into it. This is fine, since all parent functions
   // must necessarily support the instructions that are in the outlined region.
   Candidate &FirstCand = OF.Candidates.front();
   const Function &ParentFn = FirstCand.getMF()->getFunction();
   if (ParentFn.hasFnAttribute("target-features"))
     F->addFnAttr(ParentFn.getFnAttribute("target-features"));
 
+  // Set nounwind, so we don't generate eh_frame.
+  if (llvm::all_of(OF.Candidates, [](const outliner::Candidate &C) {
+        return C.getMF()->getFunction().hasFnAttribute(Attribute::NoUnwind);
+      }))
+    F->addFnAttr(Attribute::NoUnwind);
+
   BasicBlock *EntryBB = BasicBlock::Create(C, "entry", F);
   IRBuilder<> Builder(EntryBB);
   Builder.CreateRetVoid();
 
   MachineModuleInfo &MMI = getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
   MachineFunction &MF = MMI.getOrCreateMachineFunction(*F);
   MachineBasicBlock &MBB = *MF.CreateMachineBasicBlock();
   const TargetSubtargetInfo &STI = MF.getSubtarget();

llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll

 ; V8A:                  hint #31
 ; V83A:                 autibsp
 ; V8A-NEXT, V83A-NEXT:  ret
   ret void
 }
 
 ; CHECK:            [[OUTLINED_FUNC]]
 ; CHECK:            // %bb.0:
-; CHECK-NEXT:           .cfi_b_key_frame
 ; V8A-NEXT:             hint #27
 ; V83A-NEXT:            pacibsp
-; CHECK-NEXT:           .cfi_negate_ra_state
 ; V8A:                  hint #31
 ; V83A:                 autibsp
 ; V8A-NEXT, V83A-NEXT:  ret

llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-same-key-b.ll

; V83A:             pacibsp
   store i32 6, i32* %6, align 4
 ; V8A:              hint #31
 ; V83A:             autibsp
   ret void
 }
 
 ; CHECK-LABEL:      OUTLINED_FUNCTION_0:
 ; CHECK:            // %bb.0:
-; CHECK-NEXT:           .cfi_b_key_frame
 ; V8A-NEXT:             hint #27
 ; V83A-NEXT:            pacibsp
-; CHECK-NEXT:           .cfi_negate_ra_state
 ; V8A:                  hint #31
 ; V83A:                 autibsp
 ; CHECK-NEXT:           ret

llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-v8-3.ll

 
 attributes #0 = { "sign-return-address"="all"
                   "sign-return-address-key"="b_key"
                   "target-features"="+v8.3a"
                   nounwind }
 
 ; CHECK:            OUTLINED_FUNC
 ; CHECK:            // %bb.0:
-; CHECK-NEXT:               .cfi_b_key_frame
 ; CHECK-NEXT:               pacibsp
 ; CHECK:                    retab
 ; CHECK-NOT:                auti[a,b]sp

llvm/test/CodeGen/AArch64/machine-outliner-throw.ll

+; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-arm-none-eabi < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-arm-none-eabi -stop-after=machine-outliner < %s | FileCheck %s -check-prefix=TARGET_FEATURES
+
+; Make sure that we haven't added nouwind.
+; TARGET_FEATURES: define internal void @OUTLINED_FUNCTION_0()
+; TARGET_FEATURES-SAME: #[[ATTR_NUM:[0-9]+]]
+; TARGET_FEATURES: attributes #[[ATTR_NUM]] = { minsize optsize }
+
+define dso_local i32 @_Z5func1i(i32 %x) #0 {
+; CHECK-LABEL: _Z5func1i:
+; CHECK:    stp x30, x19, [sp, #-16]!
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    .cfi_offset w19, -8
+; CHECK-NEXT:    .cfi_offset w30, -16
+; CHECK-NEXT:    orr w8, wzr, #0x1
+; CHECK-NEXT:    madd w19, w0, w0, w8
+; CHECK-NEXT:    mov w0, #4
+; CHECK-NEXT:    bl __cxa_allocate_exception
+; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
+entry:
+  %mul = mul nsw i32 %x, %x
+  %add = add nuw nsw i32 %mul, 1
+  %exception = tail call i8* @__cxa_allocate_exception(i64 4) #1
+  %0 = bitcast i8* %exception to i32*
+  store i32 %add, i32* %0, align 16
+  tail call void @__cxa_throw(i8* %exception, i8* bitcast (i8** @_ZTIi to i8*), i8* null) #2
+  unreachable
+}
+
+define dso_local i32 @_Z5func2c(i8 %x) #0 {
+; CHECK-LABEL: _Z5func2c:
+; CHECK:    stp x30, x19, [sp, #-16]!
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    .cfi_offset w19, -8
+; CHECK-NEXT:    .cfi_offset w30, -16
+; CHECK-NEXT:    and w8, w0, #0xff
+; CHECK-NEXT:    orr w9, wzr, #0x1
+; CHECK-NEXT:    mov w0, #4
+; CHECK-NEXT:    madd w19, w8, w8, w9
+; CHECK-NEXT:    bl __cxa_allocate_exception
+; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
+entry:
+  %conv = zext i8 %x to i32
+  %mul = mul nuw nsw i32 %conv, %conv
+  %add = add nuw nsw i32 %mul, 1
+  %exception = tail call i8* @__cxa_allocate_exception(i64 4) #1
+  %0 = bitcast i8* %exception to i32*
+  store i32 %add, i32* %0, align 16
+  tail call void @__cxa_throw(i8* %exception, i8* bitcast (i8** @_ZTIi to i8*), i8* null) #2
+  unreachable
+}
+
+; CHECK-LABEL: OUTLINED_FUNCTION_0:
+; CHECK:      .cfi_startproc
+; CHECK:        adrp    x1, _ZTIi
+; CHECK-NEXT:   add     x1, x1, :lo12:_ZTIi
+; CHECK-NEXT:   mov     x2, xzr
+; CHECK-NEXT:   str     w19, [x0]
+; CHECK-NEXT:   b       __cxa_throw
+; CHECK:      .cfi_endproc
+
+
+@_ZTIi = external dso_local constant i8*
+declare dso_local i8* @__cxa_allocate_exception(i64) local_unnamed_addr
+declare dso_local void @__cxa_throw(i8*, i8*, i8*) local_unnamed_addr
+
+attributes #0 = { minsize noreturn optsize }
+attributes #1 = { nounwind }
+attributes #2 = { noreturn }

llvm/test/CodeGen/AArch64/machine-outliner-throw2.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-arm-none-eabi -frame-pointer=non-leaf < %s | FileCheck %s --check-prefix=NOOMIT
+; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-arm-none-eabi -frame-pointer=none < %s | FileCheck %s --check-prefix=OMITFP
+
+; FIXME: Something is wrong with the OMITFP version.
+
+define void @_Z1giii(i32 %x, i32 %y, i32 %z) minsize {
+; NOOMIT-LABEL: _Z1giii:
+; NOOMIT:       // %bb.0: // %entry
+; NOOMIT-NEXT:    b _Z1hiii
+;
+; OMITFP-LABEL: _Z1giii:
+; OMITFP:       // %bb.0: // %entry
+; OMITFP-NEXT:    b _Z1hiii
+entry:
+  tail call void @_Z1hiii(i32 %x, i32 %y, i32 %z)
+  ret void
+}
+
+declare void @_Z1hiii(i32, i32, i32) minsize
+
+define void @_Z2f1v() minsize {
+; NOOMIT-LABEL: _Z2f1v:
+; NOOMIT:       // %bb.0: // %entry
+; NOOMIT-NEXT:    stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; NOOMIT-NEXT:    mov x29, sp
+; NOOMIT-NEXT:    .cfi_def_cfa w29, 16
+; NOOMIT-NEXT:    .cfi_offset w30, -8
+; NOOMIT-NEXT:    .cfi_offset w29, -16
+; NOOMIT-NEXT:    bl OUTLINED_FUNCTION_0
+; NOOMIT-NEXT:    ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; NOOMIT-NEXT:    b _Z1giii
+;
+; OMITFP-LABEL: _Z2f1v:
+; OMITFP:       // %bb.0: // %entry
+; OMITFP-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
+; OMITFP-NEXT:    .cfi_def_cfa_offset 16
+; OMITFP-NEXT:    .cfi_offset w30, -16
+; OMITFP-NEXT:    bl OUTLINED_FUNCTION_0
+; OMITFP-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
+; OMITFP-NEXT:    b _Z1giii
+entry:
+  tail call void @_Z1giii(i32 1, i32 2, i32 3)
+  tail call void @_Z1giii(i32 1, i32 2, i32 3)
+  ret void
+}
+
+define void @_Z2f2v() minsize {
+; NOOMIT-LABEL: _Z2f2v:
+; NOOMIT:       // %bb.0: // %entry
+; NOOMIT-NEXT:    stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; NOOMIT-NEXT:    mov x29, sp
+; NOOMIT-NEXT:    .cfi_def_cfa w29, 16
+; NOOMIT-NEXT:    .cfi_offset w30, -8
+; NOOMIT-NEXT:    .cfi_offset w29, -16
+; NOOMIT-NEXT:    bl OUTLINED_FUNCTION_0
+; NOOMIT-NEXT:    ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; NOOMIT-NEXT:    b _Z1giii
+;
+; OMITFP-LABEL: _Z2f2v:
+; OMITFP:       // %bb.0: // %entry
+; OMITFP-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
+; OMITFP-NEXT:    .cfi_def_cfa_offset 16
+; OMITFP-NEXT:    .cfi_offset w30, -16
+; OMITFP-NEXT:    bl OUTLINED_FUNCTION_0
+; OMITFP-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
+; OMITFP-NEXT:    b _Z1giii
+entry:
+  tail call void @_Z1giii(i32 1, i32 2, i32 3)
+  tail call void @_Z1giii(i32 1, i32 2, i32 3)
+  ret void
+}
+
+; OMITFP-LABEL: OUTLINED_FUNCTION_0:
+; OMITFP:              .cfi_startproc
+; OMITFP-NEXT: // %bb.0:
+; OMITFP-NEXT:         .cfi_def_cfa_offset -16
+; OMITFP-NEXT:         .cfi_offset w30, 16
+; OMITFP-NEXT:         str     x30, [sp, #-16]!
+; OMITFP-NEXT:         mov     w0, #1
+; OMITFP-NEXT:         mov     w1, #2
+; OMITFP-NEXT:         mov     w2, #3
+; OMITFP-NEXT:         bl      _Z1giii
+; OMITFP-NEXT:         mov     w0, #1
+; OMITFP-NEXT:         mov     w1, #2
+; OMITFP-NEXT:         mov     w2, #3
+; OMITFP-NEXT:         ldr     x30, [sp], #16
+; OMITFP-NEXT:         ret
+
+; NOOMIT-LABEL: OUTLINED_FUNCTION_0:
+; NOOMIT:              .cfi_startproc
+; NOOMIT-NEXT: // %bb.0:
+; NOOMIT-NEXT:         .cfi_def_cfa_offset -16
+; NOOMIT-NEXT:         .cfi_offset w30, 16
+; NOOMIT-NEXT:         str     x30, [sp, #-16]!
+; NOOMIT-NEXT:         mov     w0, #1
+; NOOMIT-NEXT:         mov     w1, #2
+; NOOMIT-NEXT:         mov     w2, #3
+; NOOMIT-NEXT:         bl      _Z1giii
+; NOOMIT-NEXT:         mov     w0, #1
+; NOOMIT-NEXT:         mov     w1, #2
+; NOOMIT-NEXT:         mov     w2, #3
+; NOOMIT-NEXT:         ldr     x30, [sp], #16
+; NOOMIT-NEXT:         ret
 No newline at end of file

llvm/test/CodeGen/AArch64/machine-outliner.ll

 ; RUN: llc -verify-machineinstrs -enable-machine-outliner -aarch64-load-store-renaming=true -mtriple=aarch64-apple-darwin < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -enable-machine-outliner -aarch64-load-store-renaming=true -mtriple=aarch64-apple-darwin -mcpu=cortex-a53 -enable-misched=false < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -enable-machine-outliner -enable-linkonceodr-outlining -mtriple=aarch64-apple-darwin < %s | FileCheck %s -check-prefix=ODR
 ; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-apple-darwin -stop-after=machine-outliner < %s | FileCheck %s -check-prefix=TARGET_FEATURES
 
 ; Make sure that we inherit target features from functions and make sure we have
 ; the right function attributes.
 ; TARGET_FEATURES: define internal void @OUTLINED_FUNCTION_{{[0-9]+}}()
 ; TARGET_FEATURES-SAME: #[[ATTR_NUM:[0-9]+]]
 ; TARGET_FEATURES-DAG: attributes #[[ATTR_NUM]] = {
 ; TARGET_FEATURES-SAME: minsize
+; TARGET_FEATURES-SAME: nounwind
 ; TARGET_FEATURES-SAME: optsize
 ; TARGET_FEATURES-SAME: "target-features"="+sse"
 
 define linkonce_odr void @fish() #0 {
   ; CHECK-LABEL: _fish:
   ; CHECK-NOT: OUTLINED
   ; ODR: [[OUTLINED:OUTLINED_FUNCTION_[0-9]+]]
   %1 = alloca i32, align 4

 ; CHECK-DAG: mov     w8, #4
 ; CHECK-DAG: stp     w8, w9, [sp, #16]
 ; CHECK-DAG: mov     w9, #5
 ; CHECK-DAG: mov     w8, #6
 ; CHECK-DAG: stp     w8, w9, [sp, #8]
 ; CHECK-DAG: add     sp, sp, #32
 ; CHECK-DAG: ret
 
-attributes #0 = { noredzone "target-cpu"="cyclone" "target-features"="+sse" }
+attributes #0 = { nounwind noredzone "target-cpu"="cyclone" "target-features"="+sse" }