From a62c7951a598ae61a938bd934ac7a733b06beed4 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 21 Jan 2015 11:39:16 +1100 Subject: [PATCH] Make the slice iterator optimise better. Due to the null pointer optimisation, LLVM was leaving totally useless null pointer checks in tight loops using this iterator. We compile `Some(a_reference)` to a no-op, and matching against None in an `Option<&T>` is just comparing against 0, this means that the `None` branch in `match Some(a_reference) { None => ... }` (as appears effectively in a loop using `slice::Iter`) can't be eliminated unless `a_reference` is known to be non-null. LLVM currently doesn't seem to understand this in every situation (often due to inlining, I am told), particularly painfully in ones involving this iterator, but using `assume` means it understands almost always. for i in x { test::black_box(i); } Before: .LBB0_4: leaq 4(%rcx), %rsi testq %rcx, %rcx je .LBB0_5 movq %rcx, (%rsp) #APP #NO_APP cmpq %rsi, %rax movq %rsi, %rcx jne .LBB0_4 After: .LBB0_4: movq %rcx, (%rsp) addq $4, %rcx #APP #NO_APP cmpq %rcx, %rax jne .LBB0_4 This leads to significantly better unrolling and vectorisation too. --- src/libcore/slice.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 22da168911daa..7366109f300a8 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -40,6 +40,7 @@ use cmp::{Ordering, PartialEq, PartialOrd, Eq, Ord}; use cmp::Ordering::{Less, Equal, Greater}; use cmp; use default::Default; +use intrinsics; use iter::*; use marker::Copy; use num::Int; @@ -690,6 +691,13 @@ macro_rules! iterator { let old = self.ptr; self.ptr = self.ptr.offset(1); + // LLVM doesn't properly recognise that + // the `&` is never null without this, and + // so doesn't see that the Option we + // create here is always Some, due to the + // null-pointer optimisation. + intrinsics::assume(!old.is_null()); + Some(transmute(old)) } } @@ -723,6 +731,8 @@ macro_rules! iterator { } else { self.end = self.end.offset(-1); + // see above + intrinsics::assume(!self.end.is_null()); Some(transmute(self.end)) } }