Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command line option --no-trap-after-noreturn #67051

Merged
merged 2 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions llvm/lib/CodeGen/LLVMTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ static cl::opt<bool>
EnableTrapUnreachable("trap-unreachable", cl::Hidden,
cl::desc("Enable generating trap for unreachable"));

static cl::opt<bool> EnableNoTrapAfterNoreturn(
"no-trap-after-noreturn", cl::Hidden,
cl::desc("Do not emit a trap instruction for 'unreachable' IR instructions "
"after noreturn calls, even if --trap-unreachable is set."));

void LLVMTargetMachine::initAsmInfo() {
MRI.reset(TheTarget.createMCRegInfo(getTargetTriple().str()));
assert(MRI && "Unable to create reg info");
Expand Down Expand Up @@ -95,6 +100,8 @@ LLVMTargetMachine::LLVMTargetMachine(const Target &T,

if (EnableTrapUnreachable)
this->Options.TrapUnreachable = true;
if (EnableNoTrapAfterNoreturn)
this->Options.NoTrapAfterNoreturn = true;
}

TargetTransformInfo
Expand Down
34 changes: 31 additions & 3 deletions llvm/test/CodeGen/ARM/trap-unreachable.ll
Original file line number Diff line number Diff line change
@@ -1,8 +1,36 @@
; RUN: llc -mtriple=thumbv7 -trap-unreachable < %s | FileCheck %s
; CHECK: .inst.n 0xdefe
; RUN: llc -mtriple=thumbv7 -trap-unreachable < %s | FileCheck %s --check-prefixes CHECK,TRAP_UNREACHABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please use update_llc_test_checks.py for this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used it to generate the test originally, but had to hand-edit it because it produced this output:

...
define void @test_ntanr_noreturn() {
; TRAP_UNREACHABLE-LABEL: test_ntanr_noreturn:
; TRAP_UNREACHABLE:       @ %bb.0:
; TRAP_UNREACHABLE-NEXT:    push {r7, lr}
; TRAP_UNREACHABLE-NEXT:    bl no_return
; TRAP_UNREACHABLE-NEXT:    .inst.n 0xdefe
;
; NTANR-LABEL: test_ntanr_noreturn:
; NTANR:       @ %bb.0:
; NTANR-NEXT:    push {r7, lr}
; NTANR-NEXT:    bl no_return
  call void @no_return()
  unreachable
}

This is a bad test, because it wouldn't reject when --no-trap-after-noreturn doesn't do its job and a trap is still emitted at the end.

This was something that I came across in the other pull request too: update_llc_test_checks.py isn't very good at checking for extraneous instructions at the end of functions.

; RUN: llc -mtriple=thumbv7 -trap-unreachable -no-trap-after-noreturn < %s | FileCheck %s --check-prefixes CHECK,NTANR

define void @test() #0 {
define void @test_trap_unreachable() #0 {
; CHECK-LABEL: test_trap_unreachable:
; CHECK: @ %bb.0:
; CHECK-NEXT: .inst.n 0xdefe
unreachable
}

attributes #0 = { nounwind }

declare void @no_return() noreturn
declare void @could_return()

define void @test_ntanr_noreturn() {
; CHECK-LABEL: test_ntanr_noreturn:
; CHECK: @ %bb.0:
; CHECK-NEXT: push {r7, lr}
; CHECK-NEXT: bl no_return
; TRAP_UNREACHABLE-NEXT: .inst.n 0xdefe
; NTANR-NOT: .inst.n 0xdefe
;
call void @no_return()
unreachable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest adding another block to this function with a non-noreturn call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a new function instead, because I thought that was simpler. It makes sure --no-trap-after-noreturn isn't removing traps after normal function call, hopefully that's what you were getting at.

}

define void @test_ntanr_could_return() {
; CHECK-LABEL: test_ntanr_could_return:
; CHECK: @ %bb.0:
; CHECK-NEXT: push {r7, lr}
; CHECK-NEXT: bl could_return
; CHECK-NEXT: .inst.n 0xdefe
call void @could_return()
unreachable
}