Skip to content

Commit

Permalink
Auto merge of #36490 - bluss:zip-slightly-despecialized-edition, r=al…
Browse files Browse the repository at this point in the history
…excrichton

Remove data structure specialization for .zip() iterator

Go back on half the specialization, the part that changed the Zip
struct's fields themselves depending on the types of the iterators.

Previous PR: #33090

This means that the Zip iterator will always carry two usize fields,
which are sometimes unused. If a whole for loop using a .zip() iterator is
inlined, these are simply removed and have no effect.

The same improvement for Zip of for example slice iterators remain, and
they still optimize well. However, like when the specialization of zip
was merged, the compiler is still very sensistive to the exact context.

For example this code only autovectorizes if the function is used, not
if the code in zip_sum_i32 is inserted inline where it was called:

```rust
fn zip_sum_i32(xs: &[i32], ys: &[i32]) -> i32 {
    let mut s = 0;
    for (&x, &y) in xs.iter().zip(ys) {
        s += x * y;
    }
    s
}

fn zipdot_i32_default_zip(b: &mut test::Bencher)
{
    let xs = vec![1; 1024];
    let ys = vec![1; 1024];

    b.iter(|| {
        zip_sum_i32(&xs, &ys)
    })
}
```

Include a test that checks that `Zip<T, U>` is covariant w.r.t. T and U.

Fixes #35727
  • Loading branch information
bors authored Sep 17, 2016
2 parents cde61ba + af1a3ff commit fb62f4d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 38 deletions.
52 changes: 14 additions & 38 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,9 @@ impl<A, B> FusedIterator for Chain<A, B>
pub struct Zip<A, B> {
a: A,
b: B,
spec: <(A, B) as ZipImplData>::Data,
// index and len are only used by the specialized version of zip
index: usize,
len: usize,
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -685,17 +687,6 @@ trait ZipImpl<A, B> {
B: DoubleEndedIterator + ExactSizeIterator;
}

// Zip specialization data members
#[doc(hidden)]
trait ZipImplData {
type Data: 'static + Clone + Default + fmt::Debug;
}

#[doc(hidden)]
impl<T> ZipImplData for T {
default type Data = ();
}

// General Zip impl
#[doc(hidden)]
impl<A, B> ZipImpl<A, B> for Zip<A, B>
Expand All @@ -706,7 +697,8 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
Zip {
a: a,
b: b,
spec: Default::default(), // unused
index: 0, // unused
len: 0, // unused
}
}

Expand Down Expand Up @@ -759,20 +751,6 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
}
}

#[doc(hidden)]
#[derive(Default, Debug, Clone)]
struct ZipImplFields {
index: usize,
len: usize,
}

#[doc(hidden)]
impl<A, B> ZipImplData for (A, B)
where A: TrustedRandomAccess, B: TrustedRandomAccess
{
type Data = ZipImplFields;
}

#[doc(hidden)]
impl<A, B> ZipImpl<A, B> for Zip<A, B>
where A: TrustedRandomAccess, B: TrustedRandomAccess
Expand All @@ -782,18 +760,16 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
Zip {
a: a,
b: b,
spec: ZipImplFields {
index: 0,
len: len,
}
index: 0,
len: len,
}
}

#[inline]
fn next(&mut self) -> Option<(A::Item, B::Item)> {
if self.spec.index < self.spec.len {
let i = self.spec.index;
self.spec.index += 1;
if self.index < self.len {
let i = self.index;
self.index += 1;
unsafe {
Some((self.a.get_unchecked(i), self.b.get_unchecked(i)))
}
Expand All @@ -804,7 +780,7 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let len = self.spec.len - self.spec.index;
let len = self.len - self.index;
(len, Some(len))
}

Expand All @@ -813,9 +789,9 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
where A: DoubleEndedIterator + ExactSizeIterator,
B: DoubleEndedIterator + ExactSizeIterator
{
if self.spec.index < self.spec.len {
self.spec.len -= 1;
let i = self.spec.len;
if self.index < self.len {
self.len -= 1;
let i = self.len;
unsafe {
Some((self.a.get_unchecked(i), self.b.get_unchecked(i)))
}
Expand Down
17 changes: 17 additions & 0 deletions src/test/run-pass/variance-iterators-in-libcore.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(warnings)]

use std::iter::Zip;

fn zip_covariant<'a, A, B>(iter: Zip<&'static A, &'static B>) -> Zip<&'a A, &'a B> { iter }

fn main() { }

0 comments on commit fb62f4d

Please sign in to comment.