Auto merge of #112511 - saethlin:inline-private-functions, r=<try>

Increase the precision of the exportable MIR check r? `@ghost`
rust-lang · Jun 11, 2023 · a279a33 · a279a33
2 parents 7b6093e + 622821a
commit a279a33
Show file tree

Hide file tree

Showing 17 changed files with 195 additions and 124 deletions.
diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs
@@ -184,6 +184,7 @@ impl<'tcx> Inliner<'tcx> {
 
         self.check_mir_is_available(caller_body, &callsite.callee)?;
         let callee_body = try_instance_mir(self.tcx, callsite.callee.def)?;
+        self.check_mir_is_exportable(&callsite.callee, callee_attrs, callee_body)?;
         self.check_mir_body(callsite, callee_body, callee_attrs)?;
 
         if !self.tcx.consider_optimizing(|| {
@@ -353,6 +354,44 @@ impl<'tcx> Inliner<'tcx> {
         None
     }
 
+    fn check_mir_is_exportable(
+        &self,
+        callee: &Instance<'tcx>,
+        callee_attrs: &CodegenFnAttrs,
+        callee_body: &Body<'tcx>,
+    ) -> Result<(), &'static str> {
+        if !callee.def_id().is_local() {
+            return Ok(());
+        }
+        if self.tcx.is_constructor(callee.def_id()) {
+            return Ok(());
+        }
+        if callee_attrs.requests_inline() {
+            return Ok(());
+        }
+        let is_generic = callee.substs.non_erasable_generics().next().is_some();
+        if is_generic {
+            return Ok(());
+        }
+
+        // So now we are trying to inline a function from another crate which is not inline and is
+        // not a generic. This might work. But if this pulls in a symbol from the other crater, we
+        // will fail to link.
+        // So this is our heuritic for handling this:
+        // If this function has any calls in it, that might reference an internal symbol.
+        // If this function contains a pointer constant, that might be a reference to an internal
+        // static.
+        // So if either of those conditions are met, we cannot inline this.
+        if self.tcx.mir_inliner_callees(callee.def).is_empty()
+            && !contains_pointer_constant(callee_body)
+        {
+            debug!("Has no calls and no pointer constants, must be exportable");
+            return Ok(());
+        }
+
+        Err("not exported")
+    }
+
     /// Returns an error if inlining is not possible based on codegen attributes alone. A success
     /// indicates that inlining decision should be based on other criteria.
     fn check_codegen_attributes(
@@ -364,16 +403,6 @@ impl<'tcx> Inliner<'tcx> {
             return Err("never inline hint");
         }
 
-        // Only inline local functions if they would be eligible for cross-crate
-        // inlining. This is to ensure that the final crate doesn't have MIR that
-        // reference unexported symbols
-        if callsite.callee.def_id().is_local() {
-            let is_generic = callsite.callee.substs.non_erasable_generics().next().is_some();
-            if !is_generic && !callee_attrs.requests_inline() {
-                return Err("not exported");
-            }
-        }
-
         if callsite.fn_sig.c_variadic() {
             return Err("C variadic");
         }
@@ -1147,3 +1176,25 @@ fn try_instance_mir<'tcx>(
         _ => Ok(tcx.instance_mir(instance)),
     }
 }
+
+fn contains_pointer_constant(body: &Body<'_>) -> bool {
+    let mut finder = ConstFinder { found: false };
+    finder.visit_body(body);
+    finder.found
+}
+
+struct ConstFinder {
+    found: bool,
+}
+
+impl Visitor<'_> for ConstFinder {
+    fn visit_constant(&mut self, constant: &Constant<'_>, location: Location) {
+        debug!("Visiting constant: {:?}", constant.literal);
+        if let ConstantKind::Val(_val, ty) = constant.literal {
+            if ty.is_any_ptr() || ty.is_fn() {
+                self.found = true;
+            }
+        }
+        self.super_constant(constant, location);
+    }
+}
diff --git a/tests/codegen-units/item-collection/items-within-generic-items.rs b/tests/codegen-units/item-collection/items-within-generic-items.rs
@@ -3,6 +3,7 @@
 #![deny(dead_code)]
 #![feature(start)]
 
+#[inline(never)]
 fn generic_fn<T>(a: T) -> (T, i32) {
     //~ MONO_ITEM fn generic_fn::nested_fn
     fn nested_fn(a: i32) -> i32 {

diff --git a/tests/codegen/call-metadata.rs b/tests/codegen/call-metadata.rs
@@ -12,6 +12,7 @@ pub fn test() {
 }
 
 #[no_mangle]
+#[inline(never)]
 fn some_true() -> Option<bool> {
     Some(true)
 }
diff --git a/tests/codegen/cold-call-declare-and-call.rs b/tests/codegen/cold-call-declare-and-call.rs
@@ -9,6 +9,7 @@
 // CHECK: call coldcc void @this_should_never_happen(i16
 
 #[no_mangle]
+#[inline(never)]
 pub extern "rust-cold" fn this_should_never_happen(x: u16) {}
 
 pub fn do_things(x: u16) {

diff --git a/tests/codegen/drop.rs b/tests/codegen/drop.rs
@@ -11,6 +11,7 @@ impl Drop for SomeUniqueName {
     }
 }
 
+#[inline(never)]
 pub fn possibly_unwinding() {
 }
 

diff --git a/tests/codegen/personality_lifetimes.rs b/tests/codegen/personality_lifetimes.rs
@@ -13,6 +13,7 @@ impl Drop for S {
     }
 }
 
+#[inline(never)]
 fn might_unwind() {
 }
 

diff --git a/tests/codegen/target-cpu-on-functions.rs b/tests/codegen/target-cpu-on-functions.rs
@@ -15,7 +15,8 @@ pub extern "C" fn exported() {
 
 // CHECK-LABEL: ; target_cpu_on_functions::not_exported
 // CHECK-NEXT: ; Function Attrs:
-// CHECK-NEXT: define {{.*}}() {{.*}} #0
+// CHECK-NEXT: define {{.*}}() {{.*}} #1
+#[inline(never)]
 fn not_exported() {}
 
 // CHECK: attributes #0 = {{.*}} "target-cpu"="{{.*}}"
diff --git a/tests/codegen/tune-cpu-on-functions.rs b/tests/codegen/tune-cpu-on-functions.rs
@@ -15,7 +15,8 @@ pub extern fn exported() {
 
 // CHECK-LABEL: ; tune_cpu_on_functions::not_exported
 // CHECK-NEXT: ; Function Attrs:
-// CHECK-NEXT: define {{.*}}() {{.*}} #0
+// CHECK-NEXT: define {{.*}}() {{.*}} #1
+#[inline(never)]
 fn not_exported() {}
 
 // CHECK: attributes #0 = {{.*}} "tune-cpu"="{{.*}}"
diff --git a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.diff b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.diff
@@ -4,30 +4,19 @@
   fn main() -> () {
       let mut _0: ();                      // return place in scope 0 at $DIR/union.rs:+0:11: +0:11
       let _1: main::Un;                    // in scope 0 at $DIR/union.rs:+5:9: +5:11
-      let mut _2: u32;                     // in scope 0 at $DIR/union.rs:+5:23: +5:28
-      let mut _3: u32;                     // in scope 0 at $DIR/union.rs:+7:10: +7:26
       scope 1 {
           debug un => _1;                  // in scope 1 at $DIR/union.rs:+5:9: +5:11
           scope 2 {
           }
-          scope 3 (inlined std::mem::drop::<u32>) { // at $DIR/union.rs:16:5: 16:27
-              debug _x => _3;              // in scope 3 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
+          scope 4 (inlined std::mem::drop::<u32>) { // at $DIR/union.rs:16:5: 16:27
+              debug _x => const 1_u32;     // in scope 4 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
           }
       }
+      scope 3 (inlined val) {              // at $DIR/union.rs:14:23: 14:28
+      }
 
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/union.rs:+5:9: +5:11
-          StorageLive(_2);                 // scope 0 at $DIR/union.rs:+5:23: +5:28
-          _2 = val() -> bb1;               // scope 0 at $DIR/union.rs:+5:23: +5:28
-                                           // mir::Constant
-                                           // + span: $DIR/union.rs:14:23: 14:26
-                                           // + literal: Const { ty: fn() -> u32 {val}, val: Value(<ZST>) }
-      }
-
-      bb1: {
-          StorageDead(_2);                 // scope 0 at $DIR/union.rs:+5:29: +5:30
-          StorageLive(_3);                 // scope 1 at $DIR/union.rs:+7:10: +7:26
-          StorageDead(_3);                 // scope 1 at $DIR/union.rs:+7:26: +7:27
           StorageDead(_1);                 // scope 0 at $DIR/union.rs:+8:1: +8:2
           return;                          // scope 0 at $DIR/union.rs:+8:2: +8:2
       }

diff --git a/tests/mir-opt/inline/issue_106141.outer.Inline.diff b/tests/mir-opt/inline/issue_106141.outer.Inline.diff
@@ -4,52 +4,29 @@
   fn outer() -> usize {
       let mut _0: usize;                   // return place in scope 0 at $DIR/issue_106141.rs:+0:19: +0:24
 +     scope 1 (inlined inner) {            // at $DIR/issue_106141.rs:3:5: 3:12
-+         let mut _1: &[bool; 1];          // in scope 1 at $DIR/issue_106141.rs:12:18: 12:25
-+         let mut _2: bool;                // in scope 1 at $DIR/issue_106141.rs:14:8: 14:21
-+         let mut _3: bool;                // in scope 1 at $DIR/issue_106141.rs:14:8: 14:21
 +         scope 2 {
 +             debug buffer => const _;     // in scope 2 at $DIR/issue_106141.rs:12:9: 12:15
++             let _1: usize;               // in scope 2 at $DIR/issue_106141.rs:13:9: 13:14
 +             scope 3 {
-+                 debug index => _0;       // in scope 3 at $DIR/issue_106141.rs:13:9: 13:14
++                 debug index => _1;       // in scope 3 at $DIR/issue_106141.rs:13:9: 13:14
++             }
++             scope 4 (inlined index) {    // at $DIR/issue_106141.rs:13:17: 13:24
 +             }
 +         }
 +     }
 
       bb0: {
 -         _0 = inner() -> bb1;             // scope 0 at $DIR/issue_106141.rs:+1:5: +1:12
-+         StorageLive(_1);                 // scope 0 at $DIR/issue_106141.rs:+1:5: +1:12
-+         _1 = const _;                    // scope 1 at $DIR/issue_106141.rs:12:18: 12:25
-                                           // mir::Constant
+-                                          // mir::Constant
 -                                          // + span: $DIR/issue_106141.rs:3:5: 3:10
 -                                          // + literal: Const { ty: fn() -> usize {inner}, val: Value(<ZST>) }
-+                                          // + span: $DIR/issue_106141.rs:12:18: 12:25
-+                                          // + literal: Const { ty: &[bool; 1], val: Unevaluated(inner, [], Some(promoted[0])) }
-+         _0 = index() -> bb1;             // scope 2 at $DIR/issue_106141.rs:13:17: 13:24
-+                                          // mir::Constant
-+                                          // + span: $DIR/issue_106141.rs:13:17: 13:22
-+                                          // + literal: Const { ty: fn() -> usize {index}, val: Value(<ZST>) }
++         StorageLive(_1);                 // scope 2 at $DIR/issue_106141.rs:13:9: 13:14
++         goto -> bb1;                     // scope 2 at $DIR/issue_106141.rs:13:17: 13:24
       }
 
       bb1: {
-+         StorageLive(_3);                 // scope 3 at $DIR/issue_106141.rs:14:8: 14:21
-+         _2 = Lt(_0, const 1_usize);      // scope 3 at $DIR/issue_106141.rs:14:8: 14:21
-+         assert(move _2, "index out of bounds: the length is {} but the index is {}", const 1_usize, _0) -> bb2; // scope 3 at $DIR/issue_106141.rs:14:8: 14:21
-+     }
-+ 
-+     bb2: {
-+         _3 = (*_1)[_0];                  // scope 3 at $DIR/issue_106141.rs:14:8: 14:21
-+         switchInt(move _3) -> [0: bb3, otherwise: bb4]; // scope 3 at $DIR/issue_106141.rs:14:8: 14:21
-+     }
-+ 
-+     bb3: {
-+         _0 = const 0_usize;              // scope 3 at $DIR/issue_106141.rs:17:9: 17:10
-+         goto -> bb4;                     // scope 3 at $DIR/issue_106141.rs:14:5: 18:6
-+     }
-+ 
-+     bb4: {
-+         StorageDead(_3);                 // scope 3 at $DIR/issue_106141.rs:18:5: 18:6
-+         StorageDead(_1);                 // scope 0 at $DIR/issue_106141.rs:+1:5: +1:12
-          return;                          // scope 0 at $DIR/issue_106141.rs:+2:2: +2:2
+-         return;                          // scope 0 at $DIR/issue_106141.rs:+2:2: +2:2
++         goto -> bb1;                     // scope 4 at $DIR/issue_106141.rs:7:5: 7:12
       }
   }
 
diff --git a/tests/mir-opt/inline/issue_78442.bar.Inline.diff b/tests/mir-opt/inline/issue_78442.bar.Inline.diff
@@ -8,61 +8,55 @@
       let mut _3: &fn() {foo};             // in scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
       let _4: fn() {foo};                  // in scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
       let mut _5: ();                      // in scope 0 at $DIR/issue_78442.rs:+4:5: +4:17
-+     scope 1 (inlined <fn() {foo} as Fn<()>>::call - shim(fn() {foo})) { // at $DIR/issue_78442.rs:11:5: 11:17
++     scope 1 (inlined hide_foo) {         // at $DIR/issue_78442.rs:11:5: 11:15
++     }
++     scope 2 (inlined <fn() {foo} as Fn<()>>::call - shim(fn() {foo})) { // at $DIR/issue_78442.rs:11:5: 11:17
++         scope 3 (inlined foo) {          // at $SRC_DIR/core/src/ops/function.rs:LL:COL
++         }
 +     }
 
       bb0: {
           StorageLive(_2);                 // scope 0 at $DIR/issue_78442.rs:+4:5: +4:17
           StorageLive(_3);                 // scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
           StorageLive(_4);                 // scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
 -         _4 = hide_foo() -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
-+         _4 = hide_foo() -> [return: bb1, unwind: bb3]; // scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
-                                           // mir::Constant
-                                           // + span: $DIR/issue_78442.rs:11:5: 11:13
-                                           // + literal: Const { ty: fn() -> impl Fn() {hide_foo}, val: Value(<ZST>) }
-      }
-
-      bb1: {
+-                                          // mir::Constant
+-                                          // + span: $DIR/issue_78442.rs:11:5: 11:13
+-                                          // + literal: Const { ty: fn() -> impl Fn() {hide_foo}, val: Value(<ZST>) }
+-     }
+- 
+-     bb1: {
           _3 = &_4;                        // scope 0 at $DIR/issue_78442.rs:+4:5: +4:15
           StorageLive(_5);                 // scope 0 at $DIR/issue_78442.rs:+4:5: +4:17
           _5 = ();                         // scope 0 at $DIR/issue_78442.rs:+4:5: +4:17
 -         _2 = <fn() {foo} as Fn<()>>::call(move _3, move _5) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/issue_78442.rs:+4:5: +4:17
 -                                          // mir::Constant
 -                                          // + span: $DIR/issue_78442.rs:11:5: 11:15
 -                                          // + literal: Const { ty: for<'a> extern "rust-call" fn(&'a fn() {foo}, ()) -> <fn() {foo} as FnOnce<()>>::Output {<fn() {foo} as Fn<()>>::call}, val: Value(<ZST>) }
-+         _2 = move (*_3)() -> [return: bb5, unwind: bb3]; // scope 1 at $SRC_DIR/core/src/ops/function.rs:LL:COL
-      }
-
-      bb2: {
--         StorageDead(_5);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
--         StorageDead(_3);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
--         StorageDead(_4);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
--         StorageDead(_2);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
--         _0 = const ();                   // scope 0 at $DIR/issue_78442.rs:+3:3: +5:2
+-     }
+- 
+-     bb2: {
+          StorageDead(_5);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
+          StorageDead(_3);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
+          StorageDead(_4);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
+          StorageDead(_2);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
+          _0 = const ();                   // scope 0 at $DIR/issue_78442.rs:+3:3: +5:2
 -         drop(_1) -> [return: bb3, unwind: bb5]; // scope 0 at $DIR/issue_78442.rs:+5:1: +5:2
-+         return;                          // scope 0 at $DIR/issue_78442.rs:+5:2: +5:2
++         drop(_1) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/issue_78442.rs:+5:1: +5:2
       }
 
 -     bb3: {
--         return;                          // scope 0 at $DIR/issue_78442.rs:+5:2: +5:2
-+     bb3 (cleanup): {
-+         drop(_1) -> [return: bb4, unwind terminate]; // scope 0 at $DIR/issue_78442.rs:+5:1: +5:2
++     bb1: {
+          return;                          // scope 0 at $DIR/issue_78442.rs:+5:2: +5:2
       }
 
-      bb4 (cleanup): {
+-     bb4 (cleanup): {
 -         drop(_1) -> [return: bb5, unwind terminate]; // scope 0 at $DIR/issue_78442.rs:+5:1: +5:2
-+         resume;                          // scope 0 at $DIR/issue_78442.rs:+0:1: +5:2
-      }
-
+-     }
+- 
 -     bb5 (cleanup): {
--         resume;                          // scope 0 at $DIR/issue_78442.rs:+0:1: +5:2
-+     bb5: {
-+         StorageDead(_5);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
-+         StorageDead(_3);                 // scope 0 at $DIR/issue_78442.rs:+4:16: +4:17
-+         StorageDead(_4);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
-+         StorageDead(_2);                 // scope 0 at $DIR/issue_78442.rs:+4:17: +4:18
-+         _0 = const ();                   // scope 0 at $DIR/issue_78442.rs:+3:3: +5:2
-+         drop(_1) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/issue_78442.rs:+5:1: +5:2
++     bb2 (cleanup): {
+          resume;                          // scope 0 at $DIR/issue_78442.rs:+0:1: +5:2
       }
   }
 
diff --git a/tests/mir-opt/inline/private_helper.outer.Inline.diff b/tests/mir-opt/inline/private_helper.outer.Inline.diff
@@ -0,0 +1,21 @@
+- // MIR for `outer` before Inline
++ // MIR for `outer` after Inline
+
+  fn outer() -> u8 {
+      let mut _0: u8;                      // return place in scope 0 at $DIR/private_helper.rs:+0:19: +0:21
+- 
+-     bb0: {
+-         _0 = helper() -> bb1;            // scope 0 at $DIR/private_helper.rs:+1:5: +1:13
+-                                          // mir::Constant
+-                                          // + span: $DIR/private_helper.rs:7:5: 7:11
+-                                          // + literal: Const { ty: fn() -> u8 {helper}, val: Value(<ZST>) }
++     scope 1 (inlined helper) {           // at $DIR/private_helper.rs:7:5: 7:13
+      }
+
+-     bb1: {
++     bb0: {
++         _0 = const 123_u8;               // scope 1 at $DIR/private_helper.rs:11:5: 11:8
+          return;                          // scope 0 at $DIR/private_helper.rs:+2:2: +2:2
+      }
+  }
+
diff --git a/tests/mir-opt/inline/private_helper.rs b/tests/mir-opt/inline/private_helper.rs
@@ -0,0 +1,12 @@
+// compile-flags: -Zmir-opt-level=2 -Zinline-mir
+
+#![crate_type = "lib"]
+
+// EMIT_MIR private_helper.outer.Inline.diff
+pub fn outer() -> u8 {
+    helper()
+}
+
+fn helper() -> u8 {
+    123
+}