From 92e91f7541af929010638355dc16daf27fd28b65 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 18 Sep 2019 21:36:30 -0700 Subject: [PATCH 1/2] 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); From 6ac64abd6d4fdd60c6ee34fb1a14d2eaa9faed16 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 22 Sep 2019 21:14:54 -0700 Subject: [PATCH 2/2] Just delete the overrides now that they match the default implementations --- src/libcore/slice/mod.rs | 63 +--------------------------------------- 1 file changed, 1 insertion(+), 62 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 59f95cff5aee1..ccdab1dcf0dc9 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -28,7 +28,7 @@ use crate::fmt; use crate::intrinsics::{assume, exact_div, unchecked_sub, is_aligned_and_not_null}; use crate::isize; use crate::iter::*; -use crate::ops::{FnMut, Try, self}; +use crate::ops::{FnMut, self}; use crate::option::Option; use crate::option::Option::{None, Some}; use crate::result::Result; @@ -3180,33 +3180,6 @@ macro_rules! iterator { self.next_back() } - #[inline] - fn try_fold(&mut self, init: B, mut f: F) -> R where - Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try - { - // 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; - while let Some(x) = self.next() { - accum = f(accum, x)?; - } - Try::from_ok(accum) - } - - #[inline] - fn fold(mut self, init: Acc, mut f: Fold) -> Acc - where Fold: FnMut(Acc, Self::Item) -> Acc, - { - let mut accum = init; - while let Some(x) = self.next() { - accum = f(accum, x); - } - accum - } - #[inline] #[rustc_inherit_overflow_checks] fn position

(&mut self, mut predicate: P) -> Option where @@ -3277,40 +3250,6 @@ macro_rules! iterator { Some(next_back_unchecked!(self)) } } - - #[inline] - fn try_rfold(&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 - let mut accum = init; - unsafe { - while len!(self) >= 4 { - accum = f(accum, next_back_unchecked!(self))?; - accum = f(accum, next_back_unchecked!(self))?; - accum = f(accum, next_back_unchecked!(self))?; - accum = f(accum, next_back_unchecked!(self))?; - } - // inlining is_empty everywhere makes a huge performance difference - while !is_empty!(self) { - accum = f(accum, next_back_unchecked!(self))?; - } - } - Try::from_ok(accum) - } - - #[inline] - fn rfold(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_back() { - accum = f(accum, x); - } - accum - } } #[stable(feature = "fused", since = "1.26.0")]