diff options
| author | bors <bors@rust-lang.org> | 2021-09-13 22:14:57 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-09-13 22:14:57 +0000 |
| commit | 9f85cd6f2ab2769c16e89dcdddb3e11d9736b351 (patch) | |
| tree | 75331189ed48a97105bb5782534240c5844f7afb | |
| parent | 9bb77da74dac4768489127d21e32db19b59ada5b (diff) | |
| parent | 4d66fbc4b9b95a652636e3723937c3accec85d65 (diff) | |
| download | rust-9f85cd6f2ab2769c16e89dcdddb3e11d9736b351.tar.gz rust-9f85cd6f2ab2769c16e89dcdddb3e11d9736b351.zip | |
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
| -rw-r--r-- | compiler/rustc_target/src/abi/mod.rs | 52 | ||||
| -rw-r--r-- | src/test/assembly/niche-prefer-zero.rs | 25 | ||||
| -rw-r--r-- | src/test/ui/enum-discriminant/niche-prefer-zero.rs | 14 |
3 files changed, 82 insertions, 9 deletions
diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index af75007b09a..61607159208 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1099,19 +1099,53 @@ impl Niche { assert!(size.bits() <= 128); let max_value = size.unsigned_int_max(); - if count > max_value { + let niche = v.end.wrapping_add(1)..v.start; + let available = niche.end.wrapping_sub(niche.start) & max_value; + if count > available { return None; } - // Compute the range of invalid values being reserved. - let start = v.end.wrapping_add(1) & max_value; - let end = v.end.wrapping_add(count) & max_value; - - if v.contains(end) { - return None; + // Extend the range of valid values being reserved by moving either `v.start` or `v.end` bound. + // Given an eventual `Option<T>`, we try to maximize the chance for `None` to occupy the niche of zero. + // This is accomplished by prefering enums with 2 variants(`count==1`) and always taking the shortest path to niche zero. + // Having `None` in niche zero can enable some special optimizations. + // + // Bound selection criteria: + // 1. Select closest to zero given wrapping semantics. + // 2. Avoid moving past zero if possible. + // + // In practice this means that enums with `count > 1` are unlikely to claim niche zero, since they have to fit perfectly. + // If niche zero is already reserved, the selection of bounds are of little interest. + let move_start = |v: WrappingRange| { + let start = v.start.wrapping_sub(1) & max_value; + Some((start, Scalar { value, valid_range: v.with_start(start) })) + }; + let move_end = |v: WrappingRange| { + let start = v.end.wrapping_add(1) & max_value; + let end = v.end.wrapping_add(count) & max_value; + Some((start, Scalar { value, valid_range: v.with_end(end) })) + }; + let distance_end_zero = max_value - v.end; + if v.start > v.end { + // zero is unavailable because wrapping occurs + move_end(v) + } else if v.start <= distance_end_zero { + if count <= v.start { + move_start(v) + } else { + // moved past zero, use other bound + move_end(v) + } + } else { + let end = v.end.wrapping_add(count) & max_value; + let overshot_zero = (1..=v.end).contains(&end); + if overshot_zero { + // moved past zero, use other bound + move_start(v) + } else { + move_end(v) + } } - - Some((start, Scalar { value, valid_range: v.with_end(end) })) } } diff --git a/src/test/assembly/niche-prefer-zero.rs b/src/test/assembly/niche-prefer-zero.rs new file mode 100644 index 00000000000..0ab37a618da --- /dev/null +++ b/src/test/assembly/niche-prefer-zero.rs @@ -0,0 +1,25 @@ +// Check that niche selection prefers zero and that jumps are optimized away. +// See https://github.com/rust-lang/rust/pull/87794 +// assembly-output: emit-asm +// only-x86 +// compile-flags: -Copt-level=3 + +#![crate_type = "lib"] + +#[repr(u8)] +pub enum Size { + One = 1, + Two = 2, + Three = 3, +} + +#[no_mangle] +pub fn handle(x: Option<Size>) -> u8 { + match x { + None => 0, + Some(size) => size as u8, + } +} + +// There should be no jumps in output +// CHECK-NOT: j diff --git a/src/test/ui/enum-discriminant/niche-prefer-zero.rs b/src/test/ui/enum-discriminant/niche-prefer-zero.rs new file mode 100644 index 00000000000..f20607a8903 --- /dev/null +++ b/src/test/ui/enum-discriminant/niche-prefer-zero.rs @@ -0,0 +1,14 @@ +// Check that niche selection prefers zero. +// See https://github.com/rust-lang/rust/pull/87794 +// run-pass +#[repr(u8)] +pub enum Size { + One = 1, + Two = 2, + Three = 3, +} + +fn main() { + // check that `None` is zero + assert_eq!(0, unsafe { std::mem::transmute::<Option<Size>, u8>(None) }); +} |
