Auto merge of rust-lang#130679 - saethlin:inline-usually, r=<try>

Add inline(usually)

I'm looking into what kind of things could recover the perf improvement detected in rust-lang#121417 (comment). I think it's worth spending quite a bit of effort to figure out how to capture a 45% incr-patched improvement.

As far as I can tell, the root cause of the problem is that we have taken very deliberate steps in the compiler to ensure that `#[inline(always)]`  causes inlining where possible, even when all optimizations are disabled. Some of the reasons that was done are now outdated or were misguided. I think the only remaining use case is where the inlined body even without optimizations is cheaper to codegen or call, for example SIMD intrinsics may require a lot of code to put their arguments on the stack, which is slow to compile and run.

I'm quite sure that the majority of users applied this attribute believing it does not cause inlining in unoptimized builds, or didn't appreciate the build time regressions that implies and would prefer it didn't if they knew. (if that's you, put a heart on this or say something elsewhere, don't reply on this PR)

I am going to _try_ to use the existing benchmark suite to evaluate a number of different approaches and take notes here, and hopefully I can collect enough data to shape any conversation about what we can do to help users.

The core of this PR is `InlineAttr::Usually` (name doesn't matter) which ensures that when optimizations are enabled that the function is inlined (usual exceptions like recursion apply). I think most users believe this is what `#[inline(always)]` does.

rust-lang#130685 (comment) Replaced `#[inline(always)]` with `#[inline(usually)]` in the standard library, and did not recover the same 45% incr-patched improvement in regex. It's a tidy net positive though, and I suspect that perf improvement would normally be big enough to motivate merging a change. I think that means the standard library's use of `#[inline(always)]` is imposing marginal compile time overhead on the ecosystem, but the bigger opportunities are probably in third-party crates.

rust-lang#130679 (comment) Treats `#[inline(always)]` as `#[inline(usually)]` literally everywhere; this gets the desired incr-patched improvement but suffers quite a few check and doc regressions. I think that means that `alwaysinline` is more powerful than `function-inline-cost=0` in LLVM.

rust-lang#130679 (comment) Treats `#[inline(always)]` as `#[inline(usually)]` when `-Copt-level=0`, which looks basically the same as rust-lang#121417 (comment) (omit `alwaysinline` when doing `-Copt-level=0` codegen).

rust-lang#130679 (comment) replaces `alwaysinline` with a very negative inline cost, and it still has check and doc regressions. More investigation required on what the different inlining decision is.

rust-lang#130679 (comment) is a likely explanation of this, with some interesting implications; adding `inline(always)` to a function that was going to be inlined anyway can change change optimizations (usually it seems to improve things?).

rust-lang#130679 (comment) makes `#[inline(usually)]` also defy instantiation mode selection and always be LocalCopy the way `#[inline(always)]` does, but still has regressions in stm32f4. I think that proves that `alwaysinline` can actually improve debug build times.

rust-lang#130679 (comment) infers `alwaysinline` for extremely trivial functions, but still has regressions for stm32f4. But of course it does, I left `inline(always)` treated as `inline(usually)` which slows down the compiler 🤦 inconclusive perf run.

rust-lang#130679 (comment) doesn't have any stm32f4 regressions 🥳 I think this means that there is some threshold where `alwaysinline` produces faster debug builds.

So still two questions:
1. Why does `alwaysinline` sometimes make debug builds faster?
2. Is there any obvious threshold at which adding `alwaysinline` causes more work for debug builds?

Author: bors

compiler/rustc_attr/src/builtin.rs

@@ -46,6 +46,7 @@ pub enum InlineAttr {
     Hint,
     Always,
     Never,
+    Usually,
 }
 
 #[derive(Clone, Encodable, Decodable, Debug, PartialEq, Eq, HashStable_Generic)]

compiler/rustc_codegen_gcc/src/attributes.rs

@@ -30,6 +30,7 @@ fn inline_attr<'gcc, 'tcx>(
                 None
             }
         }
+        InlineAttr::Usually => Some(FnAttribute::Inline),
         InlineAttr::None => None,
     }
 }

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -31,14 +31,24 @@ pub(crate) fn apply_to_callsite(callsite: &Value, idx: AttributePlace, attrs: &[
 
 /// Get LLVM attribute for the provided inline heuristic.
 #[inline]
-fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll Attribute> {
+fn inline_attr<'ll>(
+    cx: &CodegenCx<'ll, '_>,
+    inline: InlineAttr,
+    should_always_inline: bool,
+) -> Option<&'ll Attribute> {
     if !cx.tcx.sess.opts.unstable_opts.inline_llvm {
         // disable LLVM inlining
         return Some(AttributeKind::NoInline.create_attr(cx.llcx));
     }
     match inline {
         InlineAttr::Hint => Some(AttributeKind::InlineHint.create_attr(cx.llcx)),
-        InlineAttr::Always => Some(AttributeKind::AlwaysInline.create_attr(cx.llcx)),
+        InlineAttr::Always => {
+            if should_always_inline {
+                Some(AttributeKind::AlwaysInline.create_attr(cx.llcx))
+            } else {
+                Some(llvm::CreateAttrStringValue(cx.llcx, "function-inline-cost", "0"))
+            }
+        }
         InlineAttr::Never => {
             if cx.sess().target.arch != "amdgpu" {
                 Some(AttributeKind::NoInline.create_attr(cx.llcx))
@@ -47,6 +57,9 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll
             }
         }
         InlineAttr::None => None,
+        InlineAttr::Usually => {
+            Some(llvm::CreateAttrStringValue(cx.llcx, "function-inline-cost", "0"))
+        }
     }
 }
 
@@ -324,6 +337,27 @@ fn create_alloc_family_attr(llcx: &llvm::Context) -> &llvm::Attribute {
     llvm::CreateAttrStringValue(llcx, "alloc-family", "__rust_alloc")
 }
 
+fn very_small_body(body: &rustc_middle::mir::Body<'_>) -> bool {
+    use rustc_middle::mir::*;
+    match body.basic_blocks.len() {
+        0 => return true,
+        1 => {}
+        2.. => return false,
+    }
+    let block = &body.basic_blocks[START_BLOCK];
+    match block.statements.len() {
+        0 => {
+            matches!(block.terminator().kind, TerminatorKind::Return)
+        }
+        1 => {
+            let statement = &block.statements[0];
+            matches!(statement.kind, StatementKind::Assign(_))
+                && matches!(block.terminator().kind, TerminatorKind::Return)
+        }
+        2.. => return false,
+    }
+}
+
 /// Helper for `FnAbi::apply_attrs_llfn`:
 /// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
 /// attributes.
@@ -353,7 +387,15 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
         } else {
             codegen_fn_attrs.inline
         };
-    to_add.extend(inline_attr(cx, inline));
+
+    let very_small_body = if cx.tcx.is_mir_available(instance.def_id()) {
+        let body = cx.tcx.instance_mir(instance.def);
+        very_small_body(body)
+    } else {
+        false
+    };
+    let should_always_inline = very_small_body || cx.tcx.sess.opts.optimize != OptLevel::No;
+    to_add.extend(inline_attr(cx, inline, should_always_inline));
 
     // The `uwtable` attribute according to LLVM is:
     //

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -528,6 +528,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                     InlineAttr::Always
                 } else if list_contains_name(items, sym::never) {
                     InlineAttr::Never
+                } else if list_contains_name(items, sym::usually) {
+                    InlineAttr::Usually
                 } else {
                     struct_span_code_err!(tcx.dcx(), items[0].span(), E0535, "invalid argument")
                         .with_help("valid inline arguments are `always` and `never`")

compiler/rustc_mir_transform/src/cross_crate_inline.rs

@@ -46,7 +46,7 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     // #[inline(never)] to force code generation.
     match codegen_fn_attrs.inline {
         InlineAttr::Never => return false,
-        InlineAttr::Hint | InlineAttr::Always => return true,
+        InlineAttr::Hint | InlineAttr::Always | InlineAttr::Usually => return true,
         _ => {}
     }
 

compiler/rustc_mir_transform/src/inline.rs

@@ -106,7 +106,7 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
         changed: false,
         caller_is_inline_forwarder: matches!(
             codegen_fn_attrs.inline,
-            InlineAttr::Hint | InlineAttr::Always
+            InlineAttr::Hint | InlineAttr::Always | InlineAttr::Usually
         ) && body_is_forwarder(body),
     };
     let blocks = START_BLOCK..body.basic_blocks.next_index();

compiler/rustc_span/src/symbol.rs

@@ -2102,6 +2102,7 @@ symbols! {
         usize_legacy_fn_max_value,
         usize_legacy_fn_min_value,
         usize_legacy_mod,
+        usually,
         va_arg,
         va_copy,
         va_end,