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.

Sign up for GitHub

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

Use call instead of invoke for functions that cannot unwind #70467

Merged
merged 1 commit into from
Apr 18, 2020
Merged

Use call instead of invoke for functions that cannot unwind #70467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/librustc_codegen_ssa/mir/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
destination: Option<(ReturnDest<'tcx, Bx::Value>, mir::BasicBlock)>,
cleanup: Option<mir::BasicBlock>,
) {
if let Some(cleanup) = cleanup {
// If there is a cleanup block and the function we're calling can unwind, then
// do an invoke, otherwise do a call.
if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) {
Copy link
Member

Choose a reason for hiding this comment

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

IMO in most instances this should happen way earlier. That is, the cleanup blocks should not be reach codegen in the first place for calls of functions that cannot unwind. This can happen either during MIR construction or afterwards in some clean-up pass.

This piece of code still needs to exist, of course, because fn_abi information in some cases may only be available after monomorphization, but the test case that changes in this PR does not involve generics...

I’m fine with work to do this happening in some follow-up PR (possibly implemented by somebody else), but in that case at least an issue needs to exist to that effect.

let ret_bx = if let Some((_, target)) = destination {
fx.blocks[target]
} else {
Expand Down
16 changes: 8 additions & 8 deletions src/test/codegen/c-variadic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,38 @@ extern "C" {
#[unwind(aborts)] // FIXME(#58794)
pub unsafe extern "C" fn use_foreign_c_variadic_0() {
// Ensure that we correctly call foreign C-variadic functions.
// CHECK: invoke void (i32, ...) @foreign_c_variadic_0([[PARAM:i32( signext)?]] 0)
// CHECK: call void (i32, ...) @foreign_c_variadic_0([[PARAM:i32( signext)?]] 0)
foreign_c_variadic_0(0);
// CHECK: invoke void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42)
// CHECK: call void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42)
foreign_c_variadic_0(0, 42i32);
// CHECK: invoke void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42, [[PARAM]] 1024)
// CHECK: call void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42, [[PARAM]] 1024)
foreign_c_variadic_0(0, 42i32, 1024i32);
// CHECK: invoke void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42, [[PARAM]] 1024, [[PARAM]] 0)
// CHECK: call void (i32, ...) @foreign_c_variadic_0([[PARAM]] 0, [[PARAM]] 42, [[PARAM]] 1024, [[PARAM]] 0)
foreign_c_variadic_0(0, 42i32, 1024i32, 0i32);
}

// Ensure that we do not remove the `va_list` passed to the foreign function when
// removing the "spoofed" `VaListImpl` that is used by Rust defined C-variadics.
#[unwind(aborts)] // FIXME(#58794)
pub unsafe extern "C" fn use_foreign_c_variadic_1_0(ap: VaList) {
// CHECK: invoke void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap)
// CHECK: call void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap)
foreign_c_variadic_1(ap);
}

#[unwind(aborts)] // FIXME(#58794)
pub unsafe extern "C" fn use_foreign_c_variadic_1_1(ap: VaList) {
// CHECK: invoke void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 42)
// CHECK: call void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 42)
foreign_c_variadic_1(ap, 42i32);
}
#[unwind(aborts)] // FIXME(#58794)
pub unsafe extern "C" fn use_foreign_c_variadic_1_2(ap: VaList) {
// CHECK: invoke void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 2, [[PARAM]] 42)
// CHECK: call void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 2, [[PARAM]] 42)
foreign_c_variadic_1(ap, 2i32, 42i32);
}

#[unwind(aborts)] // FIXME(#58794)
pub unsafe extern "C" fn use_foreign_c_variadic_1_3(ap: VaList) {
// CHECK: invoke void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 2, [[PARAM]] 42, [[PARAM]] 0)
// CHECK: call void ({{.*}}*, ...) @foreign_c_variadic_1({{.*}} %ap, [[PARAM]] 2, [[PARAM]] 42, [[PARAM]] 0)
foreign_c_variadic_1(ap, 2i32, 42i32, 0i32);
}

Expand Down
27 changes: 27 additions & 0 deletions src/test/codegen/call-llvm-intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// compile-flags: -C no-prepopulate-passes

#![feature(link_llvm_intrinsics)]
#![crate_type = "lib"]

struct A;

impl Drop for A {
fn drop(&mut self) {
println!("A");
}
}

extern {
#[link_name = "llvm.sqrt.f32"]
fn sqrt(x: f32) -> f32;
}

pub fn do_call() {
let _a = A;

unsafe {
// Ensure that we `call` LLVM intrinsics instead of trying to `invoke` them
// CHECK: call float @llvm.sqrt.f32(float 4.000000e+00
sqrt(4.0);
}
}