[llvm] [CodeGen] Do not emit TRAP for `unreachable` after `@llvm.trap` (PR #94570)
Igor Kudrin via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 9 15:29:08 PDT 2024
https://github.com/igorkudrin updated https://github.com/llvm/llvm-project/pull/94570
>From 8e02e3fd613fbbe39d68899a8d953daaa4fc0882 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Wed, 5 Jun 2024 15:18:31 -0700
Subject: [PATCH 1/2] [CodeGen] Do not emit TRAP for `unreachable` after
`@llvm.trap`
With `--trap-unreachable`, `clang` can emit double `TRAP` instructions
for code that contains a call to `__builtin_trap()`:
```
> cat test.c
void test() { __builtin_trap(); }
> 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.
---
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp | 20 ++++++-------
.../SelectionDAG/SelectionDAGBuilder.cpp | 14 +++++----
llvm/test/CodeGen/X86/trap.ll | 30 ++++++-------------
llvm/test/CodeGen/X86/unreachable-trap.ll | 16 ++++++++--
llvm/test/LTO/X86/unified-cfi.ll | 1 -
5 files changed, 42 insertions(+), 39 deletions(-)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 25b14c860284d..a9b43f03ee95c 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3076,17 +3076,17 @@ 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 (Function *F = Call->getCalledFunction();
+ F && F->getIntrinsicID() == Intrinsic::trap &&
+ !Call->hasFnAttr("trap-func-name"))
+ return true;
}
MIRBuilder.buildTrap();
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 64d6fd458c62f..f63eade302af3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3526,11 +3526,15 @@ 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 (Function *F = Call->getCalledFunction();
+ F && F->getIntrinsicID() == Intrinsic::trap &&
+ !Call->hasFnAttr("trap-func-name"))
+ 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
index 1b1837a7c5a7a..3d9a858beda85 100644
--- 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
index ee1a11c767784..d1eeab594b5b7 100644
--- a/llvm/test/CodeGen/X86/unreachable-trap.ll
+++ b/llvm/test/CodeGen/X86/unreachable-trap.ll
@@ -2,6 +2,8 @@
; RUN: llc -o - %s -mtriple=x86_64-windows-msvc | FileCheck %s --check-prefixes=CHECK,NORMAL
; 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
; CHECK-LABEL: call_exit:
; CHECK: callq {{_?}}exit
@@ -15,11 +17,19 @@ define i32 @call_exit() noreturn nounwind {
; CHECK-LABEL: trap:
; CHECK: 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
; NO_TRAP_AFTER_NORETURN-NOT: ud2
; NORMAL-NOT: ud2
-define i32 @trap() noreturn nounwind {
- tail call void @llvm.trap()
+define i32 @trap_fn_attr() noreturn nounwind {
+ tail call void @llvm.trap() #0
unreachable
}
@@ -34,3 +44,5 @@ define i32 @unreachable() noreturn nounwind {
declare void @llvm.trap() nounwind noreturn
declare void @exit(i32 %rc) nounwind noreturn
+
+attributes #0 = { "trap-func-name"="trap_func" }
diff --git a/llvm/test/LTO/X86/unified-cfi.ll b/llvm/test/LTO/X86/unified-cfi.ll
index f404136ca35f1..47dbe2b9f292a 100644
--- 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"
>From f533fd3f58ab9aecfa617d807e9ee8a70df1d737 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Sun, 9 Jun 2024 15:28:42 -0700
Subject: [PATCH 2/2] * Add `CallInst::isNonContinuableTrap()` * Replace code
repetition with a call to the new function * Handle `unreachable` after
`ubsantrap`, add a corresponding test * Add checks for `-fast-isel`
---
llvm/include/llvm/IR/Instructions.h | 11 ++++++++
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp | 4 +--
.../SelectionDAG/SelectionDAGBuilder.cpp | 4 +--
llvm/test/CodeGen/X86/unreachable-trap.ll | 5 ++--
.../test/CodeGen/X86/unreachable-ubsantrap.ll | 25 +++++++++++++++++++
5 files changed, 40 insertions(+), 9 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/unreachable-ubsantrap.ll
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index d0a051560fc9a..0fc5b198db4cc 100644
--- 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"
@@ -1836,6 +1837,16 @@ class CallInst : public CallBase {
bool canReturnTwice() const { return hasFnAttr(Attribute::ReturnsTwice); }
void setCanReturnTwice() { addFnAttr(Attribute::ReturnsTwice); }
+ 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
index a9b43f03ee95c..c5365029ded49 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3083,9 +3083,7 @@ bool IRTranslator::translateUnreachable(const User &U, MachineIRBuilder &MIRBuil
if (MF->getTarget().Options.NoTrapAfterNoreturn)
return true;
// Do not emit an additional trap instruction.
- if (Function *F = Call->getCalledFunction();
- F && F->getIntrinsicID() == Intrinsic::trap &&
- !Call->hasFnAttr("trap-func-name"))
+ if (Call->isNonContinuableTrap())
return true;
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index f63eade302af3..74b8508c4aa4b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3531,9 +3531,7 @@ void SelectionDAGBuilder::visitUnreachable(const UnreachableInst &I) {
if (DAG.getTarget().Options.NoTrapAfterNoreturn)
return;
// Do not emit an additional trap instruction.
- if (Function *F = Call->getCalledFunction();
- F && F->getIntrinsicID() == Intrinsic::trap &&
- !Call->hasFnAttr("trap-func-name"))
+ if (Call->isNonContinuableTrap())
return;
}
diff --git a/llvm/test/CodeGen/X86/unreachable-trap.ll b/llvm/test/CodeGen/X86/unreachable-trap.ll
index d1eeab594b5b7..5eb2802c61cb1 100644
--- a/llvm/test/CodeGen/X86/unreachable-trap.ll
+++ b/llvm/test/CodeGen/X86/unreachable-trap.ll
@@ -4,6 +4,7 @@
; 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
@@ -29,7 +30,7 @@ define i32 @trap() noreturn nounwind {
; NO_TRAP_AFTER_NORETURN-NOT: ud2
; NORMAL-NOT: ud2
define i32 @trap_fn_attr() noreturn nounwind {
- tail call void @llvm.trap() #0
+ tail call void @llvm.trap() "trap-func-name"="trap_func"
unreachable
}
@@ -44,5 +45,3 @@ define i32 @unreachable() noreturn nounwind {
declare void @llvm.trap() nounwind noreturn
declare void @exit(i32 %rc) nounwind noreturn
-
-attributes #0 = { "trap-func-name"="trap_func" }
diff --git a/llvm/test/CodeGen/X86/unreachable-ubsantrap.ll b/llvm/test/CodeGen/X86/unreachable-ubsantrap.ll
new file mode 100644
index 0000000000000..abe7c2274595d
--- /dev/null
+++ 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,NO_TRAP_UNREACHABLE
+; RUN: llc -global-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,NO_TRAP_UNREACHABLE
+; RUN: llc -fast-isel -o - %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,NO_TRAP_UNREACHABLE
+; 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
+; NO_TRAP_UNREACHABLE-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
More information about the llvm-commits
mailing list