Skip to content

Commit 4d4892d

Browse files
committed
Auto merge of #120193 - x17jiri:cold_match_arms, r=<try>
#[cold] on match arms ### Edit This should be in T-lang. I'm not sure how I can change it. There is discussion: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Allow.20.23.5Bcold.5D.20on.20match.20and.20if.20arms ### Summary Adds the possibility to use `#[cold]` attribute on match arms to hint the optimizer that the arm is unlikely to be taken. ### Motivation These hints are sometimes thought to help branch prediction, but the effect is probably marginal. Modern CPUs don't support hints on conditional branch instructions. They either have the current branch in the BTB (branch prediction buffer), or not, in which case the branch is predicted not to be taken. These hints are, however, helpful in letting the compiler know what is the fast path, so it can be optimized at the expense of the slow path. `grep`-ing the LLVM code for BlockFrequencyInfo and BranchProbabilityInfo shows that these hints are used at many places in the optimizer. Such as: - block placement - improve locality by making the fast path compact and move everything else out of the way - inlining, loop unrolling - these optimizations can be less aggressive on the cold path therefore reducing code size - register allocation - preferably keep in registers the data needed on the fast path ### History RFC 1131 ( #26179 ) added `likely` and `unlikely` intrinsics, which get converted to `llvm.expect.i8`. However this LLVM instruction is fragile and may get removed by some optimization passes. The problems with the intrinsics have been reported several times: #96276 , #96275 , #88767 ### Other languages Clang and GCC C++ compilers provide `__builtin_expect`. Since C++20, it is also possible to use `[[likely]]` and `[[unlikely]]` attributes. Use: ``` if (__builtin_expect(condition, false)) { ... this branch is UNlikely ... } if (condition) [[likely]] { ... this branch is likely... } ``` Note that while clang provides `__builtin_expect`, it does not convert it to `llvm.expect.i8`. Instead, it looks at the surrounding code and if there is a condition, emits branch weight metadata for conditional branches. ### Design Implementing `likely`/`unlikely` type of functions properly to emit branch weights would add significant complexity to the compiler. Additionally, these functions are not easy to use with `match` arms. Replacing the functions with attributes is easier to implement and will also work with `match`. A question remains whether these attributes should be named `likely`/`unlikely` as in C++, or if we could reuse the already existing `#[cold]` attribute. `#[cold]` has the same meaning as `unlikely`, i.e., marking the slow path, but it can currently only be used on entire functions. I personally prefer `#[cold]` because it already exists in Rust and is a short word that looks better in code. It has one disadvantage though. This code: ``` if cond #[likely] { ... } ``` becomes: ``` if cond { ... } #[cold] { ... empty cold branch ... } ``` In this PR, I implemented the possibility to add `#[cold]` attribute on match arms. Use is as follows: ``` match x { #[cold] true => { ... } // the true arm is UNlikely _ => { ... } // the false arm is likely } ``` ### Limitations The implementation only works on bool, or integers with single value arm and an otherwise arm. Extending it to other types and to `if` statements should not be too difficult.
2 parents 366d112 + 71b694c commit 4d4892d

File tree

15 files changed

+231
-21
lines changed

15 files changed

+231
-21
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -4210,6 +4210,7 @@ dependencies = [
42104210
"rustc_target",
42114211
"rustc_trait_selection",
42124212
"smallvec",
4213+
"thin-vec",
42134214
"tracing",
42144215
]
42154216

compiler/rustc_codegen_llvm/src/builder.rs

+32
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,38 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
197197
}
198198
}
199199

200+
fn cond_br_with_cold_br(
201+
&mut self,
202+
cond: &'ll Value,
203+
then_llbb: Self::BasicBlock,
204+
else_llbb: Self::BasicBlock,
205+
cold_br: Option<bool>,
206+
) {
207+
// emit the branch instruction
208+
let n = unsafe { llvm::LLVMBuildCondBr(self.llbuilder, cond, then_llbb, else_llbb) };
209+
210+
// if one of the branches is cold, emit metadata with branch weights
211+
if let Some(cold_br) = cold_br {
212+
unsafe {
213+
let s = "branch_weights";
214+
let v = [
215+
llvm::LLVMMDStringInContext(
216+
self.cx.llcx,
217+
s.as_ptr() as *const c_char,
218+
s.len() as c_uint,
219+
),
220+
self.cx.const_u32(if cold_br { 1 } else { 2000 }), // 'then' branch weight
221+
self.cx.const_u32(if cold_br { 2000 } else { 1 }), // 'else' branch weight
222+
];
223+
llvm::LLVMSetMetadata(
224+
n,
225+
llvm::MD_prof as c_uint,
226+
llvm::LLVMMDNodeInContext(self.cx.llcx, v.as_ptr(), v.len() as c_uint),
227+
);
228+
}
229+
}
230+
}
231+
200232
fn switch(
201233
&mut self,
202234
v: &'ll Value,

compiler/rustc_codegen_ssa/src/mir/block.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -327,18 +327,24 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
327327
let (test_value, target) = target_iter.next().unwrap();
328328
let lltrue = helper.llbb_with_cleanup(self, target);
329329
let llfalse = helper.llbb_with_cleanup(self, targets.otherwise());
330+
let cold_br =
331+
targets.cold_target().and_then(|t| if t == 0 { Some(true) } else { Some(false) });
332+
330333
if switch_ty == bx.tcx().types.bool {
331334
// Don't generate trivial icmps when switching on bool.
332335
match test_value {
333-
0 => bx.cond_br(discr.immediate(), llfalse, lltrue),
334-
1 => bx.cond_br(discr.immediate(), lltrue, llfalse),
336+
0 => {
337+
let cold_br = cold_br.and_then(|t| Some(!t));
338+
bx.cond_br_with_cold_br(discr.immediate(), llfalse, lltrue, cold_br);
339+
}
340+
1 => bx.cond_br_with_cold_br(discr.immediate(), lltrue, llfalse, cold_br),
335341
_ => bug!(),
336342
}
337343
} else {
338344
let switch_llty = bx.immediate_backend_type(bx.layout_of(switch_ty));
339345
let llval = bx.const_uint_big(switch_llty, test_value);
340346
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
341-
bx.cond_br(cmp, lltrue, llfalse);
347+
bx.cond_br_with_cold_br(cmp, lltrue, llfalse, cold_br);
342348
}
343349
} else if self.cx.sess().opts.optimize == OptLevel::No
344350
&& target_iter.len() == 2

compiler/rustc_codegen_ssa/src/traits/builder.rs

+7
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ pub trait BuilderMethods<'a, 'tcx>:
6464
then_llbb: Self::BasicBlock,
6565
else_llbb: Self::BasicBlock,
6666
);
67+
fn cond_br_with_cold_br(
68+
&mut self,
69+
cond: Self::Value,
70+
then_llbb: Self::BasicBlock,
71+
else_llbb: Self::BasicBlock,
72+
cold_br: Option<bool>,
73+
);
6774
fn switch(
6875
&mut self,
6976
v: Self::Value,

compiler/rustc_data_structures/src/stable_hasher.rs

+11
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::fmt;
66
use std::hash::{BuildHasher, Hash, Hasher};
77
use std::marker::PhantomData;
88
use std::mem;
9+
use thin_vec::ThinVec;
910

1011
#[cfg(test)]
1112
mod tests;
@@ -499,6 +500,16 @@ where
499500
}
500501
}
501502

503+
impl<T, CTX> HashStable<CTX> for ThinVec<T>
504+
where
505+
T: HashStable<CTX>,
506+
{
507+
#[inline]
508+
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
509+
self[..].hash_stable(ctx, hasher);
510+
}
511+
}
512+
502513
impl<T: ?Sized + HashStable<CTX>, CTX> HashStable<CTX> for Box<T> {
503514
#[inline]
504515
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {

compiler/rustc_middle/src/mir/syntax.rs

+7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use rustc_span::symbol::Symbol;
2525
use rustc_span::Span;
2626
use rustc_target::asm::InlineAsmRegOrRegClass;
2727
use smallvec::SmallVec;
28+
use thin_vec::ThinVec;
2829

2930
/// Represents the "flavors" of MIR.
3031
///
@@ -844,6 +845,12 @@ pub struct SwitchTargets {
844845
// However we’ve decided to keep this as-is until we figure a case
845846
// where some other approach seems to be strictly better than other.
846847
pub(super) targets: SmallVec<[BasicBlock; 2]>,
848+
849+
// Targets that are marked 'cold', if any.
850+
// This vector contains indices into `targets`.
851+
// It can also contain 'targets.len()' to indicate that the otherwise
852+
// branch is cold.
853+
pub(super) cold_targets: ThinVec<usize>,
847854
}
848855

849856
/// Action to be taken when a stack unwind happens.

compiler/rustc_middle/src/mir/terminator.rs

+49-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use smallvec::SmallVec;
55
use super::TerminatorKind;
66
use rustc_macros::HashStable;
77
use std::slice;
8+
use thin_vec::{thin_vec, ThinVec};
89

910
use super::*;
1011

@@ -16,13 +17,36 @@ impl SwitchTargets {
1617
pub fn new(targets: impl Iterator<Item = (u128, BasicBlock)>, otherwise: BasicBlock) -> Self {
1718
let (values, mut targets): (SmallVec<_>, SmallVec<_>) = targets.unzip();
1819
targets.push(otherwise);
19-
Self { values, targets }
20+
Self { values, targets, cold_targets: ThinVec::new() }
2021
}
2122

2223
/// Builds a switch targets definition that jumps to `then` if the tested value equals `value`,
2324
/// and to `else_` if not.
2425
pub fn static_if(value: u128, then: BasicBlock, else_: BasicBlock) -> Self {
25-
Self { values: smallvec![value], targets: smallvec![then, else_] }
26+
Self {
27+
values: smallvec![value],
28+
targets: smallvec![then, else_],
29+
cold_targets: ThinVec::new(),
30+
}
31+
}
32+
33+
/// Builds a switch targets definition that jumps to `then` if the tested value equals `value`,
34+
/// and to `else_` if not.
35+
/// If cold_br is some bool value, the given outcome is considered cold (i.e., unlikely).
36+
pub fn static_if_with_cold_br(
37+
value: u128,
38+
then: BasicBlock,
39+
else_: BasicBlock,
40+
cold_br: Option<bool>,
41+
) -> Self {
42+
Self {
43+
values: smallvec![value],
44+
targets: smallvec![then, else_],
45+
cold_targets: match cold_br {
46+
Some(br) => thin_vec![if br { 0 } else { 1 }],
47+
None => ThinVec::new(),
48+
},
49+
}
2650
}
2751

2852
/// Inverse of `SwitchTargets::static_if`.
@@ -37,6 +61,11 @@ impl SwitchTargets {
3761
}
3862
}
3963

64+
// If this switch has exactly one target, returns it.
65+
pub fn cold_target(&self) -> Option<usize> {
66+
if self.cold_targets.len() == 1 { Some(self.cold_targets[0]) } else { None }
67+
}
68+
4069
/// Returns the fallback target that is jumped to when none of the values match the operand.
4170
#[inline]
4271
pub fn otherwise(&self) -> BasicBlock {
@@ -365,6 +394,24 @@ impl<'tcx> TerminatorKind<'tcx> {
365394
TerminatorKind::SwitchInt { discr: cond, targets: SwitchTargets::static_if(0, f, t) }
366395
}
367396

397+
pub fn if_with_cold_br(
398+
cond: Operand<'tcx>,
399+
t: BasicBlock,
400+
f: BasicBlock,
401+
cold_branch: Option<bool>,
402+
) -> TerminatorKind<'tcx> {
403+
TerminatorKind::SwitchInt {
404+
discr: cond,
405+
targets: SwitchTargets::static_if_with_cold_br(
406+
0,
407+
f,
408+
t,
409+
// we compare to zero, so have to invert the branch
410+
cold_branch.and_then(|b| Some(!b)),
411+
),
412+
}
413+
}
414+
368415
#[inline]
369416
pub fn successors(&self) -> Successors<'_> {
370417
use self::TerminatorKind::*;

compiler/rustc_middle/src/thir.rs

+1
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ pub struct Arm<'tcx> {
522522
pub lint_level: LintLevel,
523523
pub scope: region::Scope,
524524
pub span: Span,
525+
pub is_cold: bool,
525526
}
526527

527528
#[derive(Copy, Clone, Debug, HashStable)]

compiler/rustc_mir_build/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ rustc_span = { path = "../rustc_span" }
2424
rustc_target = { path = "../rustc_target" }
2525
rustc_trait_selection = { path = "../rustc_trait_selection" }
2626
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
27+
thin-vec = "0.2.12"
2728
tracing = "0.1"
2829
# tidy-alphabetical-end

compiler/rustc_mir_build/src/build/expr/into.rs

+2
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8585
condition_scope,
8686
source_info,
8787
true,
88+
None,
8889
));
8990

9091
this.expr_into_dest(destination, then_blk, then)
@@ -176,6 +177,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
176177
condition_scope,
177178
source_info,
178179
true,
180+
None,
179181
)
180182
});
181183
let (short_circuit, continuation, constant) = match op {

0 commit comments

Comments
 (0)