From ff7bf792ce7d6a8650fb3596e3afad82023c5e14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 20 Oct 2023 00:00:00 +0000 Subject: [PATCH 1/3] Migrate inline_compatibility.rs test to FileCheck --- ...nlined_no_sanitize.Inline.panic-abort.diff | 21 ------- ...lined_no_sanitize.Inline.panic-unwind.diff | 21 ------- ...ned_target_feature.Inline.panic-abort.diff | 21 ------- ...ed_target_feature.Inline.panic-unwind.diff | 21 ------- ...inlined_c_variadic.Inline.panic-abort.diff | 22 -------- ...nlined_c_variadic.Inline.panic-unwind.diff | 22 -------- ...nlined_no_sanitize.Inline.panic-abort.diff | 19 ------- ...lined_no_sanitize.Inline.panic-unwind.diff | 19 ------- ...ned_target_feature.Inline.panic-abort.diff | 19 ------- ...ed_target_feature.Inline.panic-unwind.diff | 19 ------- tests/mir-opt/inline/inline_compatibility.rs | 56 +++++++++++-------- 11 files changed, 34 insertions(+), 226 deletions(-) delete mode 100644 tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-abort.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-unwind.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-abort.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-unwind.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-abort.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-unwind.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-abort.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-unwind.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-abort.diff delete mode 100644 tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-unwind.diff diff --git a/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-abort.diff deleted file mode 100644 index eac51000cac49..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-abort.diff +++ /dev/null @@ -1,21 +0,0 @@ -- // MIR for `inlined_no_sanitize` before Inline -+ // MIR for `inlined_no_sanitize` after Inline - - fn inlined_no_sanitize() -> () { - let mut _0: (); - let _1: (); -+ scope 1 (inlined no_sanitize) { -+ } - - bb0: { - StorageLive(_1); -- _1 = no_sanitize() -> [return: bb1, unwind unreachable]; -- } -- -- bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-unwind.diff deleted file mode 100644 index eba5ad9cf2694..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.panic-unwind.diff +++ /dev/null @@ -1,21 +0,0 @@ -- // MIR for `inlined_no_sanitize` before Inline -+ // MIR for `inlined_no_sanitize` after Inline - - fn inlined_no_sanitize() -> () { - let mut _0: (); - let _1: (); -+ scope 1 (inlined no_sanitize) { -+ } - - bb0: { - StorageLive(_1); -- _1 = no_sanitize() -> [return: bb1, unwind continue]; -- } -- -- bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-abort.diff deleted file mode 100644 index c2a81b9804e7e..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-abort.diff +++ /dev/null @@ -1,21 +0,0 @@ -- // MIR for `inlined_target_feature` before Inline -+ // MIR for `inlined_target_feature` after Inline - - fn inlined_target_feature() -> () { - let mut _0: (); - let _1: (); -+ scope 1 (inlined target_feature) { -+ } - - bb0: { - StorageLive(_1); -- _1 = target_feature() -> [return: bb1, unwind unreachable]; -- } -- -- bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-unwind.diff deleted file mode 100644 index 24457819b2c10..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.panic-unwind.diff +++ /dev/null @@ -1,21 +0,0 @@ -- // MIR for `inlined_target_feature` before Inline -+ // MIR for `inlined_target_feature` after Inline - - fn inlined_target_feature() -> () { - let mut _0: (); - let _1: (); -+ scope 1 (inlined target_feature) { -+ } - - bb0: { - StorageLive(_1); -- _1 = target_feature() -> [return: bb1, unwind continue]; -- } -- -- bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-abort.diff deleted file mode 100644 index 791c5a0f29f89..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-abort.diff +++ /dev/null @@ -1,22 +0,0 @@ -- // MIR for `not_inlined_c_variadic` before Inline -+ // MIR for `not_inlined_c_variadic` after Inline - - fn not_inlined_c_variadic() -> () { - let mut _0: (); - let _1: u32; - scope 1 { - debug s => _1; - } - - bb0: { - StorageLive(_1); - _1 = sum(const 4_u32, const 4_u32, const 30_u32, const 200_u32, const 1000_u32) -> [return: bb1, unwind unreachable]; - } - - bb1: { - _0 = const (); - StorageDead(_1); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-unwind.diff deleted file mode 100644 index 364acab6d9361..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.panic-unwind.diff +++ /dev/null @@ -1,22 +0,0 @@ -- // MIR for `not_inlined_c_variadic` before Inline -+ // MIR for `not_inlined_c_variadic` after Inline - - fn not_inlined_c_variadic() -> () { - let mut _0: (); - let _1: u32; - scope 1 { - debug s => _1; - } - - bb0: { - StorageLive(_1); - _1 = sum(const 4_u32, const 4_u32, const 30_u32, const 200_u32, const 1000_u32) -> [return: bb1, unwind continue]; - } - - bb1: { - _0 = const (); - StorageDead(_1); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-abort.diff deleted file mode 100644 index b9d0946b7c3a3..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-abort.diff +++ /dev/null @@ -1,19 +0,0 @@ -- // MIR for `not_inlined_no_sanitize` before Inline -+ // MIR for `not_inlined_no_sanitize` after Inline - - fn not_inlined_no_sanitize() -> () { - let mut _0: (); - let _1: (); - - bb0: { - StorageLive(_1); - _1 = no_sanitize() -> [return: bb1, unwind unreachable]; - } - - bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-unwind.diff deleted file mode 100644 index 965b7ddca3201..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.panic-unwind.diff +++ /dev/null @@ -1,19 +0,0 @@ -- // MIR for `not_inlined_no_sanitize` before Inline -+ // MIR for `not_inlined_no_sanitize` after Inline - - fn not_inlined_no_sanitize() -> () { - let mut _0: (); - let _1: (); - - bb0: { - StorageLive(_1); - _1 = no_sanitize() -> [return: bb1, unwind continue]; - } - - bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-abort.diff deleted file mode 100644 index 7c689a73482ad..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-abort.diff +++ /dev/null @@ -1,19 +0,0 @@ -- // MIR for `not_inlined_target_feature` before Inline -+ // MIR for `not_inlined_target_feature` after Inline - - fn not_inlined_target_feature() -> () { - let mut _0: (); - let _1: (); - - bb0: { - StorageLive(_1); - _1 = target_feature() -> [return: bb1, unwind unreachable]; - } - - bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-unwind.diff deleted file mode 100644 index bcdbd6e33140f..0000000000000 --- a/tests/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.panic-unwind.diff +++ /dev/null @@ -1,19 +0,0 @@ -- // MIR for `not_inlined_target_feature` before Inline -+ // MIR for `not_inlined_target_feature` after Inline - - fn not_inlined_target_feature() -> () { - let mut _0: (); - let _1: (); - - bb0: { - StorageLive(_1); - _1 = target_feature() -> [return: bb1, unwind continue]; - } - - bb1: { - StorageDead(_1); - _0 = const (); - return; - } - } - diff --git a/tests/mir-opt/inline/inline_compatibility.rs b/tests/mir-opt/inline/inline_compatibility.rs index 52f4debf5dbad..988fbe333366a 100644 --- a/tests/mir-opt/inline/inline_compatibility.rs +++ b/tests/mir-opt/inline/inline_compatibility.rs @@ -1,51 +1,63 @@ -// skip-filecheck // Checks that only functions with compatible attributes are inlined. -// // only-x86_64 -// EMIT_MIR_FOR_EACH_PANIC_STRATEGY +// compile-flags: -Cpanic=abort #![crate_type = "lib"] #![feature(no_sanitize)] #![feature(target_feature_11)] #![feature(c_variadic)] -// EMIT_MIR inline_compatibility.inlined_target_feature.Inline.diff +#[inline] +#[target_feature(enable = "sse2")] +unsafe fn sse2() {} + +#[inline] +fn nop() {} + +// CHECK-LABEL: fn f0() +// CHECK: bb0: { +// CHECK-NEXT: return; #[target_feature(enable = "sse2")] -pub unsafe fn inlined_target_feature() { - target_feature(); +pub unsafe fn f0() { + sse2(); } -// EMIT_MIR inline_compatibility.not_inlined_target_feature.Inline.diff -pub unsafe fn not_inlined_target_feature() { - target_feature(); +// CHECK-LABEL: fn f1() +// CHECK: bb0: { +// CHECK-NEXT: sse2() +pub unsafe fn f1() { + sse2(); } -// EMIT_MIR inline_compatibility.inlined_no_sanitize.Inline.diff +#[inline] +#[no_sanitize(address)] +pub unsafe fn no_sanitize() {} + +// CHECK-LABEL: fn inlined_no_sanitize() +// CHECK: bb0: { +// CHECK-NEXT: return; #[no_sanitize(address)] pub unsafe fn inlined_no_sanitize() { no_sanitize(); } -// EMIT_MIR inline_compatibility.not_inlined_no_sanitize.Inline.diff +// CHECK-LABEL: fn not_inlined_no_sanitize() +// CHECK: bb0: { +// CHECK-NEXT: no_sanitize() pub unsafe fn not_inlined_no_sanitize() { no_sanitize(); } -#[inline] -#[target_feature(enable = "sse2")] -pub unsafe fn target_feature() {} - -#[inline] -#[no_sanitize(address)] -pub unsafe fn no_sanitize() {} - -// EMIT_MIR inline_compatibility.not_inlined_c_variadic.Inline.diff +// CHECK-LABEL: fn not_inlined_c_variadic() +// CHECK: bb0: { +// CHECK-NEXT: StorageLive(_1) +// CHECK-NEXT: _1 = sum pub unsafe fn not_inlined_c_variadic() { - let s = sum(4u32, 4u32, 30u32, 200u32, 1000u32); + let _ = sum(4u32, 4u32, 30u32, 200u32, 1000u32); } -#[no_mangle] #[inline(always)] +#[no_mangle] unsafe extern "C" fn sum(n: u32, mut vs: ...) -> u32 { let mut s = 0; let mut i = 0; From b3cfd5bb04d1622b4bd35da7983e29321be18e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 20 Oct 2023 00:00:00 +0000 Subject: [PATCH 2/3] Precommit target features compatibility test --- tests/mir-opt/inline/inline_compatibility.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/mir-opt/inline/inline_compatibility.rs b/tests/mir-opt/inline/inline_compatibility.rs index 988fbe333366a..8045ea39f95ba 100644 --- a/tests/mir-opt/inline/inline_compatibility.rs +++ b/tests/mir-opt/inline/inline_compatibility.rs @@ -29,6 +29,14 @@ pub unsafe fn f1() { sse2(); } +// CHECK-LABEL: fn f2() +// CHECK: bb0: { +// CHECK-NEXT: return; +#[target_feature(enable = "avx")] +pub unsafe fn f2() { + nop(); +} + #[inline] #[no_sanitize(address)] pub unsafe fn no_sanitize() {} From 011b260cc83713599c4a3ff9bb242f92ee792b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 20 Oct 2023 00:00:00 +0000 Subject: [PATCH 3/3] Require target features to match exactly during inlining In general it is not correct to inline a callee with a target features that are subset of the callee. Require target features to match exactly during inlining. The exact match could be potentially relaxed, but this would require identifying specific feature that are allowed to differ, those that need to match, and those that can be present in caller but not in callee. This resolves MIR part of #116573. For other concerns with respect to the previous implementation also see areInlineCompatible in LLVM. --- compiler/rustc_mir_transform/src/inline.rs | 6 ++---- tests/mir-opt/inline/inline_compatibility.rs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 8b33e00c63cca..277060573bcb9 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -438,10 +438,8 @@ impl<'tcx> Inliner<'tcx> { return Err("incompatible instruction set"); } - for feature in &callee_attrs.target_features { - if !self.codegen_fn_attrs.target_features.contains(feature) { - return Err("incompatible target feature"); - } + if callee_attrs.target_features != self.codegen_fn_attrs.target_features { + return Err("incompatible target features"); } Ok(()) diff --git a/tests/mir-opt/inline/inline_compatibility.rs b/tests/mir-opt/inline/inline_compatibility.rs index 8045ea39f95ba..3ad880715fe67 100644 --- a/tests/mir-opt/inline/inline_compatibility.rs +++ b/tests/mir-opt/inline/inline_compatibility.rs @@ -31,7 +31,7 @@ pub unsafe fn f1() { // CHECK-LABEL: fn f2() // CHECK: bb0: { -// CHECK-NEXT: return; +// CHECK-NEXT: nop() #[target_feature(enable = "avx")] pub unsafe fn f2() { nop();