From 92e91f7541af929010638355dc16daf27fd28b65 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 18 Sep 2019 21:36:30 -0700 Subject: [PATCH] Remove manual unrolling from slice::Iter(Mut)::try_fold While this definitely helps sometimes (particularly for trivial closures), it's also a pessimization sometimes, so it's better to leave this to (hypothetical) future LLVM improvements instead of forcing this on everyone. I think it's better for the advice to be that sometimes you need to unroll manually than you sometimes need to not-unroll manually (like #64545). --- src/libcore/slice/mod.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 931768564ca3c..59f95cff5aee1 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3184,18 +3184,14 @@ macro_rules! iterator { fn try_fold(&mut self, init: B, mut f: F) -> R where Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try { - // manual unrolling is needed when there are conditional exits from the loop + // This method historically was unrolled, for as of 2019-09 LLVM + // is not capable of unrolling or vectorizing multiple-exit loops. + // However, doing it always proved to often be a pessimization, + // especially when called with large closures, so it was removed. + let mut accum = init; - unsafe { - while len!(self) >= 4 { - accum = f(accum, next_unchecked!(self))?; - accum = f(accum, next_unchecked!(self))?; - accum = f(accum, next_unchecked!(self))?; - accum = f(accum, next_unchecked!(self))?; - } - while !is_empty!(self) { - accum = f(accum, next_unchecked!(self))?; - } + while let Some(x) = self.next() { + accum = f(accum, x)?; } Try::from_ok(accum) } @@ -3204,8 +3200,6 @@ macro_rules! iterator { fn fold(mut self, init: Acc, mut f: Fold) -> Acc where Fold: FnMut(Acc, Self::Item) -> Acc, { - // Let LLVM unroll this, rather than using the default - // impl that would force the manual unrolling above let mut accum = init; while let Some(x) = self.next() { accum = f(accum, x);