Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve FIXME in libcore/iter/mod.rs #56630

Merged
merged 3 commits into from
Dec 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/libcore/benches/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,12 @@ bench_sums! {
(0i64..1000000).chain(1000000..).take_while(|&x| x < 1111111)
}

bench_sums! {
bench_cycle_take_sum,
bench_cycle_take_ref_sum,
(0i64..10000).cycle().take(1000000)
}

// Checks whether Skip<Zip<A,B>> is as fast as Zip<Skip<A>, Skip<B>>, from
// https://users.rust-lang.org/t/performance-difference-between-iterator-zip-and-skip-order/15743
#[bench]
Expand Down
52 changes: 30 additions & 22 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,19 @@ impl<I> Iterator for Cycle<I> where I: Clone + Iterator {
_ => (usize::MAX, None)
}
}

#[inline]
fn try_fold<Acc, F, R>(&mut self, init: Acc, mut f: F) -> R where
Self: Sized, F: FnMut(Acc, Self::Item) -> R, R: Try<Ok=Acc>
{
let mut accum = init;
while let Some(x) = self.iter.next() {
accum = f(accum, x)?;
accum = self.iter.try_fold(accum, &mut f)?;
self.iter = self.orig.clone();
}
Try::from_ok(accum)
}
}

#[stable(feature = "fused", since = "1.26.0")]
Expand Down Expand Up @@ -1855,18 +1868,11 @@ impl<I: Iterator> Iterator for Peekable<I> {

#[inline]
fn nth(&mut self, n: usize) -> Option<I::Item> {
// FIXME(#43234): merge these when borrow-checking gets better.
if n == 0 {
match self.peeked.take() {
Some(v) => v,
None => self.iter.nth(n),
}
} else {
match self.peeked.take() {
Some(None) => None,
Some(Some(_)) => self.iter.nth(n - 1),
None => self.iter.nth(n),
}
match self.peeked.take() {
Some(None) => None,
Some(v @ Some(_)) if n == 0 => v,
Some(Some(_)) => self.iter.nth(n - 1),
None => self.iter.nth(n),
}
}

Expand Down Expand Up @@ -1965,14 +1971,8 @@ impl<I: Iterator> Peekable<I> {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
pub fn peek(&mut self) -> Option<&I::Item> {
if self.peeked.is_none() {
self.peeked = Some(self.iter.next());
}
match self.peeked {
Some(Some(ref value)) => Some(value),
Some(None) => None,
_ => unreachable!(),
}
let iter = &mut self.iter;
self.peeked.get_or_insert_with(|| iter.next()).as_ref()
Copy link
Contributor Author

@sinkuu sinkuu Dec 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemingly LLVM cannot optimize out the _ => unreachable!, so this improves codegen.

Diff of LLVM IR with opt-level=3
pub fn foo(iter: &mut Peekable<vec::IntoIter<i32>>) -> Option<&i32> {
    iter.peek()
}
--- test.old.ll 2018-12-08 22:14:35.424253110 +0900
+++ test.ll     2018-12-08 22:14:43.778324073 +0900
@@ -9,73 +9,56 @@
 %"unwind::libunwind::_Unwind_Exception" = type { [0 x i64], i64, [0 x i64], void (i32, %"unwind::libunwind::_Unwind_Exception"*)*, [0 x i64], [6 x i64], [0 x i64] }
 %"unwind::libunwind::_Unwind_Context" = type { [0 x i8] }
 
-@0 = private unnamed_addr constant <{ [40 x i8] }> <{ [40 x i8] c"internal error: entered unreachable code" }>, align 1
-@1 = private unnamed_addr constant <{ [23 x i8] }> <{ [23 x i8] c"src/libcore/iter/mod.rs" }>, align 1
-@2 = private unnamed_addr constant <{ i8*, [8 x i8], i8*, [16 x i8] }> <{ i8* getelementptr inbounds (<{ [40 x i8] }>, <{ [40 x i8] }>* @0, i32 0, i32 0, i32 0), [8 x i8] c"(\00\00\00\00\00\00\00", i8* getelementptr inbounds (<{ [23 x i8] }>, <{ [23 x i8] }>* @1, i32 0, i32 0, i32 0), [16 x i8] c"\17\00\00\00\00\00\00\00\B4\07\00\00\12\00\00\00" }>, align 8
-
 ; test::foo
-; Function Attrs: nonlazybind uwtable
-define align 4 dereferenceable_or_null(4) i32* @_ZN4test3foo17h48910d518edefbc0E(%"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* dereferenceable(40) %iter) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
+; Function Attrs: norecurse nounwind nonlazybind uwtable
+define align 4 dereferenceable_or_null(4) i32* @_ZN4test3foo17h3aaa37f182f5f74fE(%"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* dereferenceable(40) %iter) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
 start:
-  %.idx.i = getelementptr %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 0
-  %.idx.val.i = load i32, i32* %.idx.i, align 4
-  %0 = icmp eq i32 %.idx.val.i, 2
-  br i1 %0, label %bb2.i, label %bb4.i
-
-bb2.i:                                            ; preds = %start
-  %1 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 7
-  %2 = load i32*, i32** %1, align 8
-  %3 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 9
-  %4 = load i32*, i32** %3, align 8
-  %5 = icmp eq i32* %2, %4
-  br i1 %5, label %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i", label %bb2.i.i
+  %0 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 0
+  %1 = load i32, i32* %0, align 4, !range !1
+  %2 = icmp eq i32 %1, 2
+  br i1 %2, label %bb2.i.i, label %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i"
+
+"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i": ; preds = %start
+  %.pre.i = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit"
 
-bb2.i.i:                                          ; preds = %bb2.i
-  %6 = getelementptr inbounds i32, i32* %2, i64 1
-  store i32* %6, i32** %1, align 8
-  %.val.i.i = load i32, i32* %2, align 4
-  br label %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i"
-
-"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i": ; preds = %bb2.i.i, %bb2.i
-  %_0.sroa.4.0.i.i = phi i32 [ %.val.i.i, %bb2.i.i ], [ undef, %bb2.i ]
-  %_0.sroa.0.0.i.i = phi i32 [ 1, %bb2.i.i ], [ 0, %bb2.i ]
-  store i32 %_0.sroa.0.0.i.i, i32* %.idx.i, align 8
-  %7 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
-  store i32 %_0.sroa.4.0.i.i, i32* %7, align 4
-  br label %bb4.i
-
-bb4.i:                                            ; preds = %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i", %start
-  %8 = phi i32 [ %_0.sroa.0.0.i.i, %"_ZN80_$LT$alloc..vec..IntoIter$LT$T$GT$$u20$as$u20$core..iter..iterator..Iterator$GT$4next17hac55a8161e3fb5c3E.exit.i" ], [ %.idx.val.i, %start ]
-  switch i32 %8, label %bb6.i [
-    i32 2, label %bb5.i
-    i32 0, label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit"
-  ]
-
-bb5.i:                                            ; preds = %bb4.i
-; call core::panicking::panic
-  tail call void @_ZN4core9panicking5panic17h2cd043862cc6dc4aE({ [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }* noalias readonly dereferenceable(40) bitcast (<{ i8*, [8 x i8], i8*, [16 x i8] }>* @2 to { [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }*))
-  unreachable
-
-bb6.i:                                            ; preds = %bb4.i
+bb2.i.i:                                          ; preds = %start
+  %3 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 7
+  %4 = load i32*, i32** %3, align 8
+  %5 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 1, i32 9
+  %6 = load i32*, i32** %5, align 8
+  %7 = icmp eq i32* %4, %6
+  br i1 %7, label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i", label %bb2.i.i.i.i
+
+bb2.i.i.i.i:                                      ; preds = %bb2.i.i
+  %8 = getelementptr inbounds i32, i32* %4, i64 1
+  store i32* %8, i32** %3, align 8
+  %.val.i.i.i.i = load i32, i32* %4, align 4
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i"
+
+"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i": ; preds = %bb2.i.i.i.i, %bb2.i.i
+  %_0.sroa.4.0.i.i.i.i = phi i32 [ %.val.i.i.i.i, %bb2.i.i.i.i ], [ undef, %bb2.i.i ]
+  %_0.sroa.0.0.i.i.i.i = phi i32 [ 1, %bb2.i.i.i.i ], [ 0, %bb2.i.i ]
+  store i32 %_0.sroa.0.0.i.i.i.i, i32* %0, align 4
   %9 = getelementptr inbounds %"core::iter::Peekable<alloc::vec::IntoIter<i32>>", %"core::iter::Peekable<alloc::vec::IntoIter<i32>>"* %iter, i64 0, i32 3, i32 1
-  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit"
+  store i32 %_0.sroa.4.0.i.i.i.i, i32* %9, align 4
+  br label %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit"
 
-"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17h89fcfd64b4417881E.exit": ; preds = %bb4.i, %bb6.i
-  %_0.0.i = phi i32* [ %9, %bb6.i ], [ null, %bb4.i ]
-  ret i32* %_0.0.i
+"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek17hd154f34ce665a0bcE.exit": ; preds = %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i", %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i"
+  %.pre-phi.i = phi i32* [ %.pre.i, %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i" ], [ %9, %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i" ]
+  %10 = phi i32 [ %1, %"start._ZN38_$LT$core..option..Option$LT$T$GT$$GT$18get_or_insert_with17h1b7503b69cd4dde4E.exit_crit_edge.i" ], [ %_0.sroa.0.0.i.i.i.i, %"_ZN38_$LT$core..iter..Peekable$LT$I$GT$$GT$4peek28_$u7b$$u7b$closure$u7d$$u7d$17h5d8c39d83231aa35E.exit.i.i" ]
+  %switch.i.i = icmp eq i32 %10, 1
+  %_0.0.i.i = select i1 %switch.i.i, i32* %.pre-phi.i, i32* null
+  ret i32* %_0.0.i.i
 }
 
 ; Function Attrs: nonlazybind uwtable
 declare i32 @rust_eh_personality(i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*) unnamed_addr #1
 
-; core::panicking::panic
-; Function Attrs: cold noinline noreturn nonlazybind uwtable
-declare void @_ZN4core9panicking5panic17h2cd043862cc6dc4aE({ [0 x i64], { [0 x i8]*, i64 }, [0 x i64], { [0 x i8]*, i64 }, [0 x i32], i32, [0 x i32], i32, [0 x i32] }* noalias readonly dereferenceable(40)) unnamed_addr #2
-
-attributes #0 = { nonlazybind uwtable "probe-stack"="__rust_probestack" }
+attributes #0 = { norecurse nounwind nonlazybind uwtable "probe-stack"="__rust_probestack" }
 attributes #1 = { nonlazybind uwtable "probe-stack"="__rust_probestack" "target-cpu"="x86-64" }
-attributes #2 = { cold noinline noreturn nonlazybind uwtable "probe-stack"="__rust_probestack" }
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 2, !"RtLibUseGOT", i32 1}
+!1 = !{i32 0, i32 3}

}
}

Expand Down Expand Up @@ -2109,8 +2109,12 @@ impl<I: Iterator, P> Iterator for TakeWhile<I, P>

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let (_, upper) = self.iter.size_hint();
(0, upper) // can't know a lower bound, due to the predicate
if self.flag {
(0, Some(0))
} else {
let (_, upper) = self.iter.size_hint();
(0, upper) // can't know a lower bound, due to the predicate
}
}

#[inline]
Expand Down Expand Up @@ -2321,6 +2325,10 @@ impl<I> Iterator for Take<I> where I: Iterator{

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
if self.n == 0 {
return (0, Some(0));
}

let (lower, upper) = self.iter.size_hint();

let lower = cmp::min(lower, self.n);
Expand Down
1 change: 1 addition & 0 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
#![feature(link_llvm_intrinsics)]
#![feature(never_type)]
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]
#![feature(exhaustive_patterns)]
#![feature(no_core)]
#![feature(on_unimplemented)]
Expand Down
2 changes: 2 additions & 0 deletions src/libcore/tests/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,8 @@ fn test_cycle() {
let mut it = (0..).step_by(1).take(0).cycle();
assert_eq!(it.size_hint(), (0, Some(0)));
assert_eq!(it.next(), None);

assert_eq!(empty::<i32>().cycle().fold(0, |acc, x| acc + x), 0);
}

#[test]
Expand Down