Repository navigation
copied().min() performance regression on nightly #140207
Description
Code
I tried this code (godbolt, from this video):
#[no_mangle]
pub fn f(a: &[u16; 8]) -> u16 {
a.iter().copied().min().unwrap()
}I expected to see this happen: compiles to vphminposuw x86 instruction (+avx feature)
Instead, this happened: compiles to unrolled cmp + cmov
Note that a manual min loop is unaffected, even with the slice iterator:
#[no_mangle]
pub fn f(a: &[u16; 8]) -> u16 {
let mut min = u16::MAX;
for &x in a {
if x < min {
min = x;
}
}
min
}Version it worked on
It most recently worked on: 1.86
Version with regression
rustc --version --verbose:
rustc 1.88.0-nightly (6bc57c6bf 2025-04-22)
binary: rustc
commit-hash: 6bc57c6bf7d0024ad9ea5a2c112f3fc9c383c8a4
commit-date: 2025-04-22
host: x86_64-unknown-linux-gnu
release: 1.88.0-nightly
LLVM version: 20.1.2
@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged
searched nightlies: from nightly-2025-02-15 to nightly-2025-04-23
regressed nightly: nightly-2025-04-16
searched commit range: 2da29db...38c560a
regressed commit: 58c2dd9
Weirdly: #139745 cc @thaliaarchi
bisected with cargo-bisect-rustc v0.6.9
Host triple: x86_64-unknown-linux-gnu
Reproduce with:
cargo bisect-rustc --start 1.86.0 -vv --without-cargo --script ./asm.sh 
Activity
thaliaarchi commentedon Apr 23, 2025
Thanks for the bisect. It indeed looks to be caused by my patch. As the linked video indicates, this instruction is limited to min of
[u16; 8]and not generalizable. Are there other affected cases?This case is the oppose of what I was attempting to optimize
Copied<T>for—where the iterated data is smaller than a pointer.For
Cloned<T>, perhaps we should specialize onT: TrivialClone(#135634, cc @joboet) to match whateverCopied<T>is changed to do.the8472 commentedon Apr 23, 2025
u8, u32, usize no longer vectorize either, probably others too. They just use(ed) different vector instructions.
It's strange that #139745 got merged considering that my similar proposal to elide side-effects in #90209 got rejected.
joboet commentedon Apr 24, 2025
@rustbot label +I-libs-nominated
I merged #139745 assuming that the precise behaviour around side-effects of the iterator methods was an implementation detail, but that should probably have been discussed beforehand. Also, it was the cause of this performance regression. T-libs, do you want to revert this change?
Amanieu commentedon Apr 30, 2025
We discussed this in the @rust-lang/libs meeting. We would like #139745 to be reverted for now and re-submitted with a libs-api nomination so that we can properly discuss the exact implications.
I believe the reason this optimizes differently is that before #139745 the
compareclosure was passed references to copies of the values yielded by the iterator, whereas #139745 changes this to pass the original references yielded by the iterator instead.2 remaining items
the8472 commentedon Apr 30, 2025
Some previous issues observing that by-reference slice iteration vectorizes poorly:
&usize::<=is much slower thanusize::<=#105259thaliaarchi commentedon May 1, 2025
By the way, I am not planning to submit the PRs for the revert or selective reintroduction, even though it's my bug. Sorry. The revert should be easy, though. I'm currently not looking to add any more PRs to my current stack.
Rollup merge of rust-lang#140644 - the8472:revert-copy-clone-adapters…
Unrolled build for rust-lang#140644
Rollup merge of rust-lang#140644 - the8472:revert-copy-clone-adapters…
Auto merge of rust-lang#140925 - the8472:test-140207, r=compiler-errors
TheElectronWill commentedon Aug 13, 2025
This seems fixed in 1.89.0 (godbolt): both iter-based and loop-based versions compile to
vphminposuwwith-C opt-level=3 -C target-feature=+avx.clubby789 commentedon Aug 13, 2025
Adding needs-test since I think it would be useful to make sure issues like this are caught if we re-land something like the original patch
DaniPopes commentedon Aug 13, 2025
Is this not #140925 ?
clubby789 commentedon Aug 13, 2025
Looks correct - closed by #140925