[CodeGen] Do not emit TRAP for unreachable after @llvm.trap (#94570)

igorkudrin · web-flow · commit 23db37c51cd3 · 2024-07-02T15:36:02.000-07:00
With `--trap-unreachable`, `clang` can emit double `TRAP` instructions
for code that contains a call to `__builtin_trap()`:

```
&gt; cat test.c
void test() { __builtin_trap(); }
&gt; clang test.c --target=x86_64 -mllvm --trap-unreachable -O1 -S -o -
...
test:
...
  ud2
  ud2
...
```

`SimplifyCFGPass` inserts `unreachable` after a call to a `noreturn`
function, and later this instruction causes `TRAP/G_TRAP` to be emitted
in `SelectionDAGBuilder::visitUnreachable()` or
`IRTranslator::translateUnreachable()` if
`TargetOptions.TrapUnreachable` is set.

The patch checks the instruction before `unreachable` and avoids
inserting an additional trap.
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
@@ -29,6 +29,7 @@
 #include "llvm/IR/GEPNoWrapFlags.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
+#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/OperandTraits.h"
 #include "llvm/IR/Use.h"
 #include "llvm/IR/User.h"
@@ -1520,6 +1521,17 @@ class CallInst : public CallBase {
   bool canReturnTwice() const { return hasFnAttr(Attribute::ReturnsTwice); }
   void setCanReturnTwice() { addFnAttr(Attribute::ReturnsTwice); }
 
+  /// Return true if the call is for a noreturn trap intrinsic.
+  bool isNonContinuableTrap() const {
+    switch (getIntrinsicID()) {
+    case Intrinsic::trap:
+    case Intrinsic::ubsantrap:
+      return !hasFnAttr("trap-func-name");
+    default:
+      return false;
+    }
+  }
+
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Instruction *I) {
     return I->getOpcode() == Instruction::Call;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3082,17 +3082,15 @@ bool IRTranslator::translateUnreachable(const User &U, MachineIRBuilder &MIRBuil
     return true;
 
   auto &UI = cast<UnreachableInst>(U);
+
   // We may be able to ignore unreachable behind a noreturn call.
-  if (MF->getTarget().Options.NoTrapAfterNoreturn) {
-    const BasicBlock &BB = *UI.getParent();
-    if (&UI != &BB.front()) {
-      BasicBlock::const_iterator PredI =
-        std::prev(BasicBlock::const_iterator(UI));
-      if (const CallInst *Call = dyn_cast<CallInst>(&*PredI)) {
-        if (Call->doesNotReturn())
-          return true;
-      }
-    }
+  if (const CallInst *Call = dyn_cast_or_null<CallInst>(UI.getPrevNode());
+      Call && Call->doesNotReturn()) {
+    if (MF->getTarget().Options.NoTrapAfterNoreturn)
+      return true;
+    // Do not emit an additional trap instruction.
+    if (Call->isNonContinuableTrap())
+      return true;
   }
 
   MIRBuilder.buildTrap();
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3541,11 +3541,13 @@ void SelectionDAGBuilder::visitUnreachable(const UnreachableInst &I) {
     return;
 
   // We may be able to ignore unreachable behind a noreturn call.
-  if (DAG.getTarget().Options.NoTrapAfterNoreturn) {
-    if (const CallInst *Call = dyn_cast_or_null<CallInst>(I.getPrevNode())) {
-      if (Call->doesNotReturn())
-        return;
-    }
+  if (const CallInst *Call = dyn_cast_or_null<CallInst>(I.getPrevNode());
+      Call && Call->doesNotReturn()) {
+    if (DAG.getTarget().Options.NoTrapAfterNoreturn)
+      return;
+    // Do not emit an additional trap instruction.
+    if (Call->isNonContinuableTrap())
+      return;
   }
 
   DAG.setRoot(DAG.getNode(ISD::TRAP, getCurSDLoc(), MVT::Other, DAG.getRoot()));
diff --git a/llvm/test/CodeGen/X86/trap.ll b/llvm/test/CodeGen/X86/trap.ll
@@ -1,33 +1,22 @@
-; RUN: llc < %s -mtriple=i686-apple-darwin8 -mcpu=yonah | FileCheck %s -check-prefix=DARWIN
-; RUN: llc < %s -mtriple=i686-unknown-linux -mcpu=yonah | FileCheck %s -check-prefix=LINUX
-; RUN: llc < %s -mtriple=x86_64-scei-ps4 | FileCheck %s -check-prefix=PS4
-; RUN: llc < %s -mtriple=x86_64-sie-ps5  | FileCheck %s -check-prefix=PS4
-; RUN: llc < %s -mtriple=x86_64-windows-msvc | FileCheck %s -check-prefix=WIN64
+; RUN: llc < %s -mtriple=i686-apple-darwin8 -mcpu=yonah | FileCheck %s -check-prefixes=CHECK,DARWIN
+; RUN: llc < %s -mtriple=i686-unknown-linux -mcpu=yonah | FileCheck %s -check-prefixes=CHECK,LINUX
+; RUN: llc < %s -mtriple=x86_64-scei-ps4 | FileCheck %s -check-prefixes=CHECK,PS4
+; RUN: llc < %s -mtriple=x86_64-sie-ps5  | FileCheck %s -check-prefixes=CHECK,PS4
+; RUN: llc < %s -mtriple=x86_64-windows-msvc | FileCheck %s -check-prefixes=CHECK,WIN64
 
-; DARWIN-LABEL: test0:
-; DARWIN: ud2
-; LINUX-LABEL: test0:
-; LINUX: ud2
-; FIXME: PS4 probably doesn't want two ud2s.
-; PS4-LABEL: test0:
-; PS4: ud2
-; PS4: ud2
-; WIN64-LABEL: test0:
-; WIN64: ud2
-; WIN64-NOT: ud2
+; CHECK-LABEL: test0:
+; CHECK: ud2
+; CHECK-NOT: ud2
 define i32 @test0() noreturn nounwind  {
 entry:
 	tail call void @llvm.trap( )
 	unreachable
 }
 
-; DARWIN-LABEL: test1:
+; CHECK-LABEL: test1:
 ; DARWIN: int3
-; LINUX-LABEL: test1:
 ; LINUX: int3
-; PS4-LABEL: test1:
 ; PS4: int     $65
-; WIN64-LABEL: test1:
 ; WIN64: int3
 ; WIN64-NOT: ud2
 define i32 @test1() noreturn nounwind  {
@@ -38,4 +27,3 @@ entry:
 
 declare void @llvm.trap() nounwind 
 declare void @llvm.debugtrap() nounwind 
-
diff --git a/llvm/test/CodeGen/X86/unreachable-trap.ll b/llvm/test/CodeGen/X86/unreachable-trap.ll
@@ -1,33 +1,51 @@
-; RUN: llc -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,NORMAL
-; RUN: llc -o - %s -mtriple=x86_64-windows-msvc | FileCheck %s --check-prefixes=CHECK,NORMAL
+; RUN: llc -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK
+; RUN: llc -o - %s -mtriple=x86_64-windows-msvc | FileCheck %s --check-prefixes=CHECK
 ; RUN: llc -o - %s -mtriple=x86_64-scei-ps4 | 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
+; RUN: llc --trap-unreachable -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_AFTER_NORETURN
+; RUN: llc --trap-unreachable -global-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_AFTER_NORETURN
+; RUN: llc --trap-unreachable -fast-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_AFTER_NORETURN
 
 ; CHECK-LABEL: call_exit:
 ; CHECK: callq {{_?}}exit
 ; TRAP_AFTER_NORETURN: ud2
-; NO_TRAP_AFTER_NORETURN-NOT: ud2
-; NORMAL-NOT: ud2
+; CHECK-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
-; NORMAL-NOT: ud2
+; CHECK-NOT: ud2
 define i32 @trap() noreturn nounwind {
   tail call void @llvm.trap()
   unreachable
 }
 
+; CHECK-LABEL: trap_fn_attr:
+; CHECK: callq {{_?}}trap_func
+; TRAP_AFTER_NORETURN: ud2
+; CHECK-NOT: ud2
+define i32 @trap_fn_attr() noreturn nounwind {
+  tail call void @llvm.trap() "trap-func-name"="trap_func"
+  unreachable
+}
+
+; CHECK-LABEL: noreturn_indirect:
+; CHECK: callq *%r{{.+}}
+; TRAP_AFTER_NORETURN: ud2
+; CHECK-NOT: ud2
+define i32 @noreturn_indirect(ptr %fptr) noreturn nounwind {
+  tail call void (...) %fptr() noreturn nounwind
+  unreachable
+}
+
 ; CHECK-LABEL: unreachable:
 ; TRAP_AFTER_NORETURN: ud2
 ; NO_TRAP_AFTER_NORETURN: ud2
-; NORMAL-NOT: ud2
-; NORMAL: # -- End function
+; CHECK-NOT: ud2
+; CHECK: # -- End function
 define i32 @unreachable() noreturn nounwind {
   unreachable
 }
diff --git a/llvm/test/CodeGen/X86/unreachable-ubsantrap.ll b/llvm/test/CodeGen/X86/unreachable-ubsantrap.ll
@@ -0,0 +1,25 @@
+; RUN: llc -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK
+; RUN: llc -global-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK
+; RUN: llc -fast-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK
+; RUN: llc --trap-unreachable -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_UNREACHABLE
+; RUN: llc --trap-unreachable -global-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_UNREACHABLE
+; RUN: llc --trap-unreachable -fast-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,TRAP_UNREACHABLE
+
+; CHECK-LABEL: ubsantrap:
+; CHECK:         ud1l 12(%eax), %eax
+; CHECK-NOT:     ud2
+define i32 @ubsantrap() noreturn nounwind {
+  call void @llvm.ubsantrap(i8 12)
+  unreachable
+}
+
+; CHECK-LABEL:      ubsantrap_fn_attr:
+; CHECK:              callq {{_?}}ubsantrap_func
+; TRAP_UNREACHABLE:   ud2
+; CHECK-NOT:          ud2
+define i32 @ubsantrap_fn_attr() noreturn nounwind {
+  call void @llvm.ubsantrap(i8 12) "trap-func-name"="ubsantrap_func"
+  unreachable
+}
+
+declare void @llvm.ubsantrap(i8) cold noreturn nounwind
diff --git a/llvm/test/LTO/X86/unified-cfi.ll b/llvm/test/LTO/X86/unified-cfi.ll
@@ -6,7 +6,6 @@
 
 ; CHECK: jbe
 ; CHECK-NEXT: ud2
-; CHECK-NEXT: ud2
 
 ; ModuleID = 'llvm/test/LTO/X86/unified-cfi.ll'
 source_filename = "cfi.cpp"
