Skip to content

Commit 9f85cd6

Browse files
committed
Auto merge of #87794 - bonega:enum_niche_prefer_zero, r=nagisa
Enum should prefer discriminant zero for niche Given an enum with unassigned zero-discriminant, rust should prefer it for niche selection. Zero as discriminant for `Option<Enum>` makes it possible for LLVM to optimize resulting asm. - Eliminate branch when expected value coincides. - Use smaller instruction `test eax, eax` instead of `cmp eax, ?` - Possible interaction with zeroed memory? Example: ```rust pub enum Size { One = 1, Two = 2, Three = 3, } pub fn handle(x: Option<Size>) -> u8 { match x { None => {0} Some(size) => {size as u8} } } ``` In this case discriminant zero is available as a niche. Above example on nightly: ```asm mov eax, edi cmp al, 4 jne .LBB0_2 xor eax, eax .LBB0_2: ret ``` PR: ```asm mov eax, edi ret ``` I created this PR because I had a performance regression when I tried to use an enum to represent legal grapheme byte-length for utf8. Using an enum instead of `NonZeroU8` [here](https://github.com/bonega/yore/blob/d683304f5dfe2e99f769e6ab8adf8d60a0d1d9b3/src/internal/decoder_incomplete.rs#L90) resulted in a performance regression of about 5%. I consider this to be a somewhat realistic benchmark. Thanks to `@ogoffart` for pointing me in the right direction! Edit: Updated description
2 parents 9bb77da + 4d66fbc commit 9f85cd6

File tree

3 files changed

+82
-9
lines changed

3 files changed

+82
-9
lines changed

compiler/rustc_target/src/abi/mod.rs

+43-9
Original file line numberDiff line numberDiff line change
@@ -1099,19 +1099,53 @@ impl Niche {
10991099
assert!(size.bits() <= 128);
11001100
let max_value = size.unsigned_int_max();
11011101

1102-
if count > max_value {
1102+
let niche = v.end.wrapping_add(1)..v.start;
1103+
let available = niche.end.wrapping_sub(niche.start) & max_value;
1104+
if count > available {
11031105
return None;
11041106
}
11051107

1106-
// Compute the range of invalid values being reserved.
1107-
let start = v.end.wrapping_add(1) & max_value;
1108-
let end = v.end.wrapping_add(count) & max_value;
1109-
1110-
if v.contains(end) {
1111-
return None;
1108+
// Extend the range of valid values being reserved by moving either `v.start` or `v.end` bound.
1109+
// Given an eventual `Option<T>`, we try to maximize the chance for `None` to occupy the niche of zero.
1110+
// This is accomplished by prefering enums with 2 variants(`count==1`) and always taking the shortest path to niche zero.
1111+
// Having `None` in niche zero can enable some special optimizations.
1112+
//
1113+
// Bound selection criteria:
1114+
// 1. Select closest to zero given wrapping semantics.
1115+
// 2. Avoid moving past zero if possible.
1116+
//
1117+
// In practice this means that enums with `count > 1` are unlikely to claim niche zero, since they have to fit perfectly.
1118+
// If niche zero is already reserved, the selection of bounds are of little interest.
1119+
let move_start = |v: WrappingRange| {
1120+
let start = v.start.wrapping_sub(1) & max_value;
1121+
Some((start, Scalar { value, valid_range: v.with_start(start) }))
1122+
};
1123+
let move_end = |v: WrappingRange| {
1124+
let start = v.end.wrapping_add(1) & max_value;
1125+
let end = v.end.wrapping_add(count) & max_value;
1126+
Some((start, Scalar { value, valid_range: v.with_end(end) }))
1127+
};
1128+
let distance_end_zero = max_value - v.end;
1129+
if v.start > v.end {
1130+
// zero is unavailable because wrapping occurs
1131+
move_end(v)
1132+
} else if v.start <= distance_end_zero {
1133+
if count <= v.start {
1134+
move_start(v)
1135+
} else {
1136+
// moved past zero, use other bound
1137+
move_end(v)
1138+
}
1139+
} else {
1140+
let end = v.end.wrapping_add(count) & max_value;
1141+
let overshot_zero = (1..=v.end).contains(&end);
1142+
if overshot_zero {
1143+
// moved past zero, use other bound
1144+
move_start(v)
1145+
} else {
1146+
move_end(v)
1147+
}
11121148
}
1113-
1114-
Some((start, Scalar { value, valid_range: v.with_end(end) }))
11151149
}
11161150
}
11171151

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Check that niche selection prefers zero and that jumps are optimized away.
2+
// See https://github.com/rust-lang/rust/pull/87794
3+
// assembly-output: emit-asm
4+
// only-x86
5+
// compile-flags: -Copt-level=3
6+
7+
#![crate_type = "lib"]
8+
9+
#[repr(u8)]
10+
pub enum Size {
11+
One = 1,
12+
Two = 2,
13+
Three = 3,
14+
}
15+
16+
#[no_mangle]
17+
pub fn handle(x: Option<Size>) -> u8 {
18+
match x {
19+
None => 0,
20+
Some(size) => size as u8,
21+
}
22+
}
23+
24+
// There should be no jumps in output
25+
// CHECK-NOT: j
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Check that niche selection prefers zero.
2+
// See https://github.com/rust-lang/rust/pull/87794
3+
// run-pass
4+
#[repr(u8)]
5+
pub enum Size {
6+
One = 1,
7+
Two = 2,
8+
Three = 3,
9+
}
10+
11+
fn main() {
12+
// check that `None` is zero
13+
assert_eq!(0, unsafe { std::mem::transmute::<Option<Size>, u8>(None) });
14+
}

0 commit comments

Comments
 (0)