Skip to content

Commit

Permalink
Auto merge of #43595 - oyvindln:master, r=aturon
Browse files Browse the repository at this point in the history
Add an overflow check in the Iter::next() impl for Range<_> to help with vectorization.

This helps with vectorization in some cases, such as (0..u16::MAX).collect::<Vec<u16>>(),
 as LLVM is able to change the loop condition to use equality instead of less than and should help with #43124. (See also my [last comment](#43124 (comment)) there.) This PR makes collect on ranges of u16, i16, i8, and u8 **significantly** faster (at least on x86-64 and i686), and pretty close, though not quite equivalent to a [manual unsafe implementation](https://is.gd/nkoecB). 32 ( and 64-bit values on x86-64) bit values were already vectorized without this change, and they still are. This PR doesn't seem to help with 64-bit values on i686, as they still don't vectorize well compared to doing a manual loop.

I'm a bit unsure if this was the best way of implementing this, I tried to do it with as little changes as possible and avoided changing the step trait and the behavior in RangeFrom (I'll leave that for others like #43127 to discuss wider changes to the trait). I tried simply changing the comparison to `self.start != self.end` though that made the compiler segfault when compiling stage0, so I went with this method instead for now.

As for `next_back()`, reverse ranges seem to optimise properly already.
  • Loading branch information
bors committed Aug 9, 2017
2 parents 78efb23 + 4bb9a8b commit 0f9317d
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions src/libcore/iter/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,16 @@ impl<A: Step> Iterator for ops::Range<A> {
#[inline]
fn next(&mut self) -> Option<A> {
if self.start < self.end {
let mut n = self.start.add_one();
mem::swap(&mut n, &mut self.start);
Some(n)
// We check for overflow here, even though it can't actually
// happen. Adding this check does however help llvm vectorize loops
// for some ranges that don't get vectorized otherwise,
// and this won't actually result in an extra check in an optimized build.
if let Some(mut n) = self.start.add_usize(1) {
mem::swap(&mut n, &mut self.start);
Some(n)
} else {
None
}
} else {
None
}
Expand Down

0 comments on commit 0f9317d

Please sign in to comment.