Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-04-27 18:57:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #150386 - saethlin:no-const-assert-likely, r=tgross35

Browse Source 
Remove the explicit branch hint from const_panic

This was asked for in PR review for equivalence with `assert!`, but we don't need likely/unlikely intrinsics here (and indeed they are a bit of an antipattern), because codegen knows about the pattern that `assert!` produces where only one target of a SwitchInt leads to a block terminated by a `#[cold]` call. All our panic entrypoints are `#[cold]`.

`assert!` does not use the likely/unlikely intrinsics, maybe it did in the past.

So the intrinsic call in `const_assert!` should be completely unnecessary. But currently it isn't, because `const_panic!` stamps out a runtime wrapper for its entrypoint which is not given `#[inline]`, citing compile times. It's not obvious to me why not using `#[inline]` would even be good in this case.

I think the runtime wrapper should have `#[inline]` or `#[cold]`, so I'm going to do 3 perf experiments here.
Just changing `const_assert!`: https://perf.rust-lang.org/compare.html?start=fabece9e9491d0a3c3655dba488881968b7c5f7a&end=f6b082cfcf880ad80b33523d5047c11654d907a8&stat=instructions:u
Also adding `#[inline]` to `const_panic!`'s runtime wrapper: https://perf.rust-lang.org/compare.html?start=fabece9e9491d0a3c3655dba488881968b7c5f7a&end=f467cc7e49b74b47da0e2cde49fc1a3140e00ff6&stat=instructions:u
Also adding `#[cold]` to `const_panic!`'s runtime wrapper: https://perf.rust-lang.org/compare.html?start=2e854a9344154564259de51385e9ec9506c0f3b7&end=fd8e098558ff08ac002b05ff78e9f91ffa6f3738&stat=instructions:u

The previous comment indicated perf would be worse with `#[inline]` on the runtime branch, but the results in this PR show that it is more mixed, and net positive. And in fact, the only regression in serde seems to be because we created a very tiny additional amount of codegen, but pushed the size of the second CGU just high enough that we no longer merge all CGUs together. So now serde (in debug with incremental disabled) has 2 CGUs instead of 1. If we just so happened to be still below the threshold where the CGUs don't merge, adding `#[inline]` would have been nearly all improvements.
This commit is contained in:
bors
2025-12-30 04:45:36 +00:00
parent b082bd59d2 315646a7a0
commit d874dce125
3 changed files with 22 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
library/core/src/intrinsics/mod.rs
+3 -25
View File
@@ -2402,8 +2402,7 @@ pub const fn const_eval_select<ARG: Tuple, F, G, RET>(
/// The `@capture` block declares which surrounding variables / expressions can be
/// used inside the `if const`.
/// Note that the two arms of this `if` really each become their own function, which is why the
/// macro supports setting attributes for those functions. The runtime function is always
/// marked as `#[inline]`.
/// macro supports setting attributes for those functions. Both functions are marked as `#[inline]`.
///
/// See [`const_eval_select()`] for the rules and requirements around that intrinsic.
pub(crate) macro const_eval_select {
@@ -2413,35 +2412,14 @@ pub const fn const_eval_select<ARG: Tuple, F, G, RET>(
$(#[$compiletime_attr:meta])* $compiletime:block
else
$(#[$runtime_attr:meta])* $runtime:block
) => {
// Use the `noinline` arm, after adding explicit `inline` attributes
$crate::intrinsics::const_eval_select!(
@capture$([$($binders)*])? { $($arg : $ty = $val),* } $(-> $ret)? :
#[noinline]
if const
#[inline] // prevent codegen on this function
$(#[$compiletime_attr])*
$compiletime
else
#[inline] // avoid the overhead of an extra fn call
$(#[$runtime_attr])*
$runtime
)
},
// With a leading #[noinline], we don't add inline attributes
(
@capture$([$($binders:tt)*])? { $($arg:ident : $ty:ty = $val:expr),* $(,)? } $( -> $ret:ty )? :
#[noinline]
if const
$(#[$compiletime_attr:meta])* $compiletime:block
else
$(#[$runtime_attr:meta])* $runtime:block
) => {{
#[inline]
$(#[$runtime_attr])*
fn runtime$(<$($binders)*>)?($($arg: $ty),*) $( -> $ret )? {
$runtime
}
#[inline]
$(#[$compiletime_attr])*
const fn compiletime$(<$($binders)*>)?($($arg: $ty),*) $( -> $ret )? {
// Don't warn if one of the arguments is unused.
library/core/src/panic.rs
+3 -4
View File
@@ -166,10 +166,9 @@ fn as_str(&mut self) -> Option<&str> {
const fn do_panic($($arg: $ty),*) -> ! {
$crate::intrinsics::const_eval_select!(
@capture { $($arg: $ty = $arg),* } -> !:
#[noinline]
if const #[track_caller] #[inline] { // Inline this, to prevent codegen
if const #[track_caller] {
$crate::panic!($const_msg)
} else #[track_caller] { // Do not inline this, it makes perf worse
} else #[track_caller] {
$crate::panic!($runtime_msg)
}
)
@@ -195,7 +194,7 @@ const fn do_panic($($arg: $ty),*) -> ! {
#[doc(hidden)]
pub macro const_assert {
($condition: expr, $const_msg:literal, $runtime_msg:literal, $($arg:tt)*) => {{
if !$crate::intrinsics::likely($condition) {
if !($condition) {
$crate::panic::const_panic!($const_msg, $runtime_msg, $($arg)*)
}
}}
tests/codegen-llvm/intrinsics/likely_assert.rs
+16 -2
View File
@@ -1,17 +1,31 @@
//@ compile-flags: -Copt-level=3
#![feature(panic_internals, const_eval_select, rustc_allow_const_fn_unstable, core_intrinsics)]
#![crate_type = "lib"]
// check that assert! and const_assert! emit branch weights
#[no_mangle]
pub fn test_assert(x: bool) {
assert!(x);
}
// check that assert! emits branch weights
// CHECK-LABEL: @test_assert(
// CHECK: br i1 %x, label %bb2, label %bb1, !prof ![[NUM:[0-9]+]]
// CHECK: bb1:
// CHECK: panic
// CHECK: bb2:
// CHECK: ret void
#[no_mangle]
pub fn test_const_assert(x: bool) {
core::panic::const_assert!(x, "", "",);
}
// CHECK-LABEL: @test_const_assert(
// CHECK: br i1 %x, label %bb2, label %bb1, !prof ![[NUM:[0-9]+]]
// CHECK: bb1:
// CHECK: panic
// CHECK: bb2:
// CHECK: ret void
// CHECK: ![[NUM]] = !{!"branch_weights", {{(!"expected", )?}}i32 2000, i32 1}