SelectionDAGBuilder, mach-o: Skip trap after noreturn call (for Mach-O)

[MatzeB]

Summary

Add NoTrapAfterNoreturn target option which skips emission of traps
behind noreturn calls even if TrapUnreachable is enabled.

rdar://41530228

[MatzeB]

update

[t.p.northover]

This looks reasonable to me.

[MatzeB]

Landed in rL335877

[dblaikie]

Out of curiosity, what's the use case/motivation for this?

[MatzeB]

In D48674#1149645, @dblaikie wrote:

Out of curiosity, what's the use case/motivation for this?

It reduces code size (0.5-1.2%) in the projects I measured.

[dblaikie]

This feature was/is only enabled in -O0, though, right? As a debugging aid,
if I recall correctly - because it can be super confusing when your code
falls off the end of one function into the next. Is the code size savings
(only at O0) worth the increased cost to debug?

[rnk]

These two MachO specific configurations seem like needlessly MachO specific policy decisions. I think it would make more sense for the project overall if we emitted traps for unreachables not preceded by noreturn calls for all targets. As you've noticed, the vast majority of unreachables are from calls to noreturn functions. The remainder account for the confusing and hard to debug fall through cases. I believe noreturn functions rarely return erroneously, and are usually not problematic for the user.

[rnk]

By the way, once upon a time I *almost* removed TrapOnUnreachable in rL248959, but I had to revert it for reasons I am trying to rediscover.

include/llvm/Target/TargetOptions.h

TargetOptions()
           NoNaNsFPMath(false), NoTrappingFPMath(false),
           NoSignedZerosFPMath(false),
           HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
           GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
           EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
           DisableIntegratedAS(false), RelaxELFRelocations(false),
           FunctionSections(false), DataSections(false),
           UniqueSectionNames(true), TrapUnreachable(false),
+          NoTrapAfterNoreturn(false),
           EmulatedTLS(false), ExplicitEmulatedTLS(false),
           EnableIPRA(false), EmitStackSizeSection(false) {}
 
     /// PrintMachineCode - This flag is enabled when the -print-machineinstrs
     /// option is specified on the command line, and should enable debugging
     /// output from the code generator.
     unsigned PrintMachineCode : 1;
 
public:
     /// Emit data into separate sections.
     unsigned DataSections : 1;
 
     unsigned UniqueSectionNames : 1;
 
     /// Emit target-specific trap instruction for 'unreachable' IR instructions.
     unsigned TrapUnreachable : 1;
 
+    /// Do not emit a trap instruction for 'unreachable' IR instructions behind
+    /// noreturn calls, even if TrapUnreachable is true.
+    unsigned NoTrapAfterNoreturn : 1;
+
     /// EmulatedTLS - This flag enables emulated TLS model, using emutls
     /// function in the runtime library..
     unsigned EmulatedTLS : 1;
 
     /// Whether -emulated-tls or -no-emulated-tls is set.
     unsigned ExplicitEmulatedTLS : 1;
 
     /// This flag enables InterProcedural Register Allocation (IPRA).

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visitIndirectBr(const IndirectBrInst &I) {
   IndirectBrMBB->normalizeSuccProbs();
 
   DAG.setRoot(DAG.getNode(ISD::BRIND, getCurSDLoc(),
                           MVT::Other, getControlRoot(),
                           getValue(I.getAddress())));
 }
 
 void SelectionDAGBuilder::visitUnreachable(const UnreachableInst &I) {
-  if (DAG.getTarget().Options.TrapUnreachable)
-    DAG.setRoot(
-        DAG.getNode(ISD::TRAP, getCurSDLoc(), MVT::Other, DAG.getRoot()));
+  if (!DAG.getTarget().Options.TrapUnreachable)
+    return;
+
+  // We may be able to ignore unreachable behind a noreturn call.
+  if (DAG.getTarget().Options.NoTrapAfterNoreturn) {
+    const BasicBlock &BB = *I.getParent();
+    if (&I != &BB.front()) {
+      BasicBlock::const_iterator PredI =
+        std::prev(BasicBlock::const_iterator(&I));
+      if (const CallInst *Call = dyn_cast<CallInst>(&*PredI)) {
+        if (Call->doesNotReturn())
+          return;
+      }
+    }
+  }
+
+  DAG.setRoot(DAG.getNode(ISD::TRAP, getCurSDLoc(), MVT::Other, DAG.getRoot()));
 }
 
 void SelectionDAGBuilder::visitFSub(const User &I) {
   // -0.0 - X --> fneg
   Type *Ty = I.getType();
   if (isa<Constant>(I.getOperand(0)) &&
       I.getOperand(0) == ConstantFP::getZeroValueForNegation(Ty)) {
     SDValue Op2 = getValue(I.getOperand(1));

lib/Target/AArch64/AArch64TargetMachine.cpp

AArch64TargetMachine::AArch64TargetMachine(const Target &T, const Triple &TT,
                                            bool LittleEndian)
     : LLVMTargetMachine(T,
                         computeDataLayout(TT, Options.MCOptions, LittleEndian),
                         TT, CPU, FS, Options, getEffectiveRelocModel(TT, RM),
                         getEffectiveCodeModel(TT, CM, JIT), OL),
       TLOF(createTLOF(getTargetTriple())), isLittle(LittleEndian) {
   initAsmInfo();
 
-  if (TT.isOSBinFormatMachO())
+  if (TT.isOSBinFormatMachO()) {
     this->Options.TrapUnreachable = true;
+    this->Options.NoTrapAfterNoreturn = true;
+  }
 
   // Enable GlobalISel at or below EnableGlobalISelAt0.
   if (getOptLevel() <= EnableGlobalISelAtO)
     setGlobalISel(true);
 }
 
 AArch64TargetMachine::~AArch64TargetMachine() = default;
 

lib/Target/ARM/ARMTargetMachine.cpp

if ((TargetTriple.getEnvironment() == Triple::GNUEABI ||
          TargetTriple.getEnvironment() == Triple::MuslEABI ||
          TargetTriple.getEnvironment() == Triple::MuslEABIHF) &&
         !(TargetTriple.isOSWindows() || TargetTriple.isOSDarwin()))
       this->Options.EABIVersion = EABI::GNU;
     else
       this->Options.EABIVersion = EABI::EABI5;
   }
 
-  if (TT.isOSBinFormatMachO())
+  if (TT.isOSBinFormatMachO()) {
     this->Options.TrapUnreachable = true;
+    this->Options.NoTrapAfterNoreturn = true;
+  }
 
   initAsmInfo();
 }
 
 ARMBaseTargetMachine::~ARMBaseTargetMachine() = default;
 
 const ARMSubtarget *
 ARMBaseTargetMachine::getSubtargetImpl(const Function &F) const {

lib/Target/X86/X86TargetMachine.cpp

X86TargetMachine::X86TargetMachine(const Target &T, const Triple &TT,
   // To prevent that, we emit a trap for 'unreachable' IR instructions.
   // (which on X86, happens to be the 'ud2' instruction)
   // On PS4, the "return address" of a 'noreturn' call must still be within
   // the calling function, and TrapUnreachable is an easy way to get that.
   // The check here for 64-bit windows is a bit icky, but as we're unlikely
   // to ever want to mix 32 and 64-bit windows code in a single module
   // this should be fine.
   if ((TT.isOSWindows() && TT.getArch() == Triple::x86_64) || TT.isPS4() ||
-      TT.isOSBinFormatMachO())
+      TT.isOSBinFormatMachO()) {
     this->Options.TrapUnreachable = true;
+    this->Options.NoTrapAfterNoreturn = TT.isOSBinFormatMachO();
+  }
 
   initAsmInfo();
 }
 
 X86TargetMachine::~X86TargetMachine() = default;
 
 const X86Subtarget *
 X86TargetMachine::getSubtargetImpl(const Function &F) const {

test/CodeGen/X86/unreachable-trap.ll

+; RUN: llc -o - %s -mtriple=x86_64-windows-msvc | FileCheck %s --check-prefixes=CHECK,TRAP_AFTER_NORETURN
+; RUN: llc -o - %s -mtriple=x86_64-apple-darwin | FileCheck %s --check-prefixes=CHECK,NO_TRAP_AFTER_NORETURN
+
+; CHECK-LABEL: call_exit:
+; CHECK: callq {{_?}}exit
+; TRAP_AFTER_NORETURN: ud2
+; NO_TRAP_AFTER_NORETURN-NOT: ud2
+define i32 @call_exit() noreturn nounwind {
+  tail call void @exit(i32 0)
+  unreachable
+}
+
+; CHECK-LABEL: trap:
+; CHECK: ud2
+; TRAP_AFTER_NORETURN: ud2
+; NO_TRAP_AFTER_NORETURN-NOT: ud2
+define i32 @trap() noreturn nounwind {
+  tail call void @llvm.trap()
+  unreachable
+}
+
+; CHECK-LABEL: unreachable:
+; CHECK: ud2
+define i32 @unreachable() noreturn nounwind {
+  unreachable
+}
+
+declare void @llvm.trap() nounwind noreturn
+declare void @exit(i32 %rc) nounwind noreturn