-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Conversation
Add the command line option --no-trap-after-noreturn, which exposes the pre-existing TargetOption NoTrapAfterNoreturn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is what I had in mind.
@@ -1,8 +1,25 @@ | |||
; 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
; NTANR-NOT: .inst.n 0xdefe | ||
; | ||
call void @no_return() | ||
unreachable |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
The CI on Windows seems to be failing because of issues with anti-virus software, I don't think it's my fault. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add the command line option --no-trap-after-noreturn, which exposes the pre-existing TargetOption
NoTrapAfterNoreturn
.This pull request was split off from this one: #65876