From af1a3ffbebef27f3dc267e66c15325596d1a2cff Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Thu, 15 Sep 2016 09:59:55 +0200 Subject: [PATCH 1/5] 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. This means that the Zip iterator will always carry two usize fields, which are 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 it was called: ``` 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 is covariant w.r.t. T and U. --- src/libcore/iter/mod.rs | 52 +++++-------------- .../run-pass/variance-iterators-in-libcore.rs | 17 ++++++ 2 files changed, 31 insertions(+), 38 deletions(-) create mode 100644 src/test/run-pass/variance-iterators-in-libcore.rs diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index b1d3ab1d1febc..dd57fd1b5190f 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -643,7 +643,9 @@ impl FusedIterator for Chain pub struct Zip { 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")] @@ -685,17 +687,6 @@ trait ZipImpl { B: DoubleEndedIterator + ExactSizeIterator; } -// Zip specialization data members -#[doc(hidden)] -trait ZipImplData { - type Data: 'static + Clone + Default + fmt::Debug; -} - -#[doc(hidden)] -impl ZipImplData for T { - default type Data = (); -} - // General Zip impl #[doc(hidden)] impl ZipImpl for Zip @@ -706,7 +697,8 @@ impl ZipImpl for Zip Zip { a: a, b: b, - spec: Default::default(), // unused + index: 0, // unused + len: 0, // unused } } @@ -759,20 +751,6 @@ impl ZipImpl for Zip } } -#[doc(hidden)] -#[derive(Default, Debug, Clone)] -struct ZipImplFields { - index: usize, - len: usize, -} - -#[doc(hidden)] -impl ZipImplData for (A, B) - where A: TrustedRandomAccess, B: TrustedRandomAccess -{ - type Data = ZipImplFields; -} - #[doc(hidden)] impl ZipImpl for Zip where A: TrustedRandomAccess, B: TrustedRandomAccess @@ -782,18 +760,16 @@ impl ZipImpl for Zip 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))) } @@ -804,7 +780,7 @@ impl ZipImpl for Zip #[inline] fn size_hint(&self) -> (usize, Option) { - let len = self.spec.len - self.spec.index; + let len = self.len - self.index; (len, Some(len)) } @@ -813,9 +789,9 @@ impl ZipImpl for Zip 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))) } diff --git a/src/test/run-pass/variance-iterators-in-libcore.rs b/src/test/run-pass/variance-iterators-in-libcore.rs new file mode 100644 index 0000000000000..b9677d5ba8598 --- /dev/null +++ b/src/test/run-pass/variance-iterators-in-libcore.rs @@ -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 or the MIT license +// , 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() { } From 6950242b1cfff94eda669f65bf082c54fde5e90f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 15 Sep 2016 19:02:10 +0200 Subject: [PATCH 2/5] Workaround #34427 by using memset of 0 on ARM to set the discriminant. --- src/librustc_trans/adt.rs | 25 +++++++++++++++++++++---- src/test/run-pass/issue-34427.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 src/test/run-pass/issue-34427.rs diff --git a/src/librustc_trans/adt.rs b/src/librustc_trans/adt.rs index e8498363e45a3..050b5127e6acf 100644 --- a/src/librustc_trans/adt.rs +++ b/src/librustc_trans/adt.rs @@ -54,6 +54,7 @@ use syntax::ast; use syntax::attr; use syntax::attr::IntType; use abi::FAT_PTR_ADDR; +use base; use build::*; use common::*; use debuginfo::DebugLoc; @@ -963,16 +964,32 @@ pub fn trans_set_discr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>, Store(bcx, C_null(llptrty), val); } } - StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => { + StructWrappedNullablePointer { nndiscr, ref discrfield, ref nonnull, .. } => { if discr != nndiscr { - let llptrptr = GEPi(bcx, val, &discrfield[..]); - let llptrty = val_ty(llptrptr).element_type(); - Store(bcx, C_null(llptrty), llptrptr); + if target_sets_discr_via_memset(bcx) { + // Issue #34427: As workaround for LLVM bug on + // ARM, use memset of 0 on whole struct rather + // than storing null to single target field. + let b = B(bcx); + let llptr = b.pointercast(val, Type::i8(b.ccx).ptr_to()); + let fill_byte = C_u8(b.ccx, 0); + let size = C_u64(b.ccx, nonnull.size); + let align = C_u32(b.ccx, nonnull.align); + base::call_memset(&b, llptr, fill_byte, size, align, false); + } else { + let llptrptr = GEPi(bcx, val, &discrfield[..]); + let llptrty = val_ty(llptrptr).element_type(); + Store(bcx, C_null(llptrty), llptrptr); + } } } } } +fn target_sets_discr_via_memset<'blk, 'tcx>(bcx: Block<'blk, 'tcx>) -> bool { + bcx.sess().target.target.arch == "arm" || bcx.sess().target.target.arch == "aarch64" +} + fn assert_discr_in_range(ity: IntType, min: Disr, max: Disr, discr: Disr) { match ity { attr::UnsignedInt(_) => { diff --git a/src/test/run-pass/issue-34427.rs b/src/test/run-pass/issue-34427.rs new file mode 100644 index 0000000000000..6bf8a2ac6a72d --- /dev/null +++ b/src/test/run-pass/issue-34427.rs @@ -0,0 +1,26 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Issue #34427: On ARM, the code in `foo` at one time was generating +// a machine code instruction of the form: `str r0, [r0, rN]!` (for +// some N), which is not legal because the source register and base +// register cannot be identical in the preindexed form signalled by +// the `!`. +// +// See LLVM bug: https://llvm.org/bugs/show_bug.cgi?id=28809 + +#[inline(never)] +fn foo(n: usize) -> Vec> { + (0..n).map(|_| None).collect() +} + +fn main() { + let _ = (foo(10), foo(32)); +} From e8a44d29b633412b4173be8f1bfb6dae7ac3c45e Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sat, 10 Sep 2016 14:33:29 -0700 Subject: [PATCH 3/5] librustc_mir: Remove `&*x` when `x` has a reference type. This introduces a new `InstCombine` pass for us to place such peephole optimizations. --- src/librustc_driver/driver.rs | 3 + src/librustc_mir/transform/instcombine.rs | 110 ++++++++++++++++++++++ src/librustc_mir/transform/mod.rs | 2 + 3 files changed, 115 insertions(+) create mode 100644 src/librustc_mir/transform/instcombine.rs diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 36e9fccdf5fd8..14cd05279b1ca 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1017,6 +1017,7 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads); passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("no-landing-pads")); + // From here on out, regions are gone. passes.push_pass(box mir::transform::erase_regions::EraseRegions); passes.push_pass(box mir::transform::add_call_guards::AddCallGuards); @@ -1024,6 +1025,8 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads); passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("elaborate-drops")); + // No lifetime analysis based on borrowing can be done from here on out. + passes.push_pass(box mir::transform::instcombine::InstCombine::new()); passes.push_pass(box mir::transform::deaggregator::Deaggregator); passes.push_pass(box mir::transform::add_call_guards::AddCallGuards); diff --git a/src/librustc_mir/transform/instcombine.rs b/src/librustc_mir/transform/instcombine.rs new file mode 100644 index 0000000000000..a0331f03b0197 --- /dev/null +++ b/src/librustc_mir/transform/instcombine.rs @@ -0,0 +1,110 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Performs various peephole optimizations. + +use rustc::mir::repr::{Location, Lvalue, Mir, Operand, ProjectionElem, Rvalue}; +use rustc::mir::transform::{MirPass, MirSource, Pass}; +use rustc::mir::visit::{MutVisitor, Visitor}; +use rustc::ty::TyCtxt; +use rustc::util::nodemap::FnvHashSet; +use std::mem; + +pub struct InstCombine { + optimizations: OptimizationList, +} + +impl InstCombine { + pub fn new() -> InstCombine { + InstCombine { + optimizations: OptimizationList::default(), + } + } +} + +impl Pass for InstCombine {} + +impl<'tcx> MirPass<'tcx> for InstCombine { + fn run_pass<'a>(&mut self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + _: MirSource, + mir: &mut Mir<'tcx>) { + // We only run when optimizing MIR (at any level). + if tcx.sess.opts.debugging_opts.mir_opt_level == Some(0) { + return + } + + // First, find optimization opportunities. This is done in a pre-pass to keep the MIR + // read-only so that we can do global analyses on the MIR in the process (e.g. + // `Lvalue::ty()`). + { + let mut optimization_finder = OptimizationFinder::new(mir, tcx); + optimization_finder.visit_mir(mir); + self.optimizations = optimization_finder.optimizations + } + + // Then carry out those optimizations. + MutVisitor::visit_mir(&mut *self, mir); + } +} + +impl<'tcx> MutVisitor<'tcx> for InstCombine { + fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, location: Location) { + if self.optimizations.and_stars.remove(&location) { + debug!("Replacing `&*`: {:?}", rvalue); + let new_lvalue = match *rvalue { + Rvalue::Ref(_, _, Lvalue::Projection(ref mut projection)) => { + mem::replace(&mut projection.base, Lvalue::ReturnPointer) + } + _ => bug!("Detected `&*` but didn't find `&*`!"), + }; + *rvalue = Rvalue::Use(Operand::Consume(new_lvalue)) + } + + self.super_rvalue(rvalue, location) + } +} + +/// Finds optimization opportunities on the MIR. +struct OptimizationFinder<'b, 'a, 'tcx:'a+'b> { + mir: &'b Mir<'tcx>, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + optimizations: OptimizationList, +} + +impl<'b, 'a, 'tcx:'b> OptimizationFinder<'b, 'a, 'tcx> { + fn new(mir: &'b Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> OptimizationFinder<'b, 'a, 'tcx> { + OptimizationFinder { + mir: mir, + tcx: tcx, + optimizations: OptimizationList::default(), + } + } +} + +impl<'b, 'a, 'tcx> Visitor<'tcx> for OptimizationFinder<'b, 'a, 'tcx> { + fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { + if let Rvalue::Ref(_, _, Lvalue::Projection(ref projection)) = *rvalue { + if let ProjectionElem::Deref = projection.elem { + if projection.base.ty(self.mir, self.tcx).to_ty(self.tcx).is_region_ptr() { + self.optimizations.and_stars.insert(location); + } + } + } + + self.super_rvalue(rvalue, location) + } +} + +#[derive(Default)] +struct OptimizationList { + and_stars: FnvHashSet, +} + diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index c3485b8256da1..e99b7a976e3e3 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -18,3 +18,5 @@ pub mod promote_consts; pub mod qualify_consts; pub mod dump_mir; pub mod deaggregator; +pub mod instcombine; + From 82639d4f0429eedd868d25d78b69a9fd96e5d200 Mon Sep 17 00:00:00 2001 From: Mikhail Modin Date: Fri, 16 Sep 2016 21:44:15 +0300 Subject: [PATCH 4/5] fix top level attr spans --- src/libsyntax/parse/attr.rs | 2 +- src/test/ui/span/issue-36530.rs | 14 ++++++++++++++ src/test/ui/span/issue-36530.stderr | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/span/issue-36530.rs create mode 100644 src/test/ui/span/issue-36530.stderr diff --git a/src/libsyntax/parse/attr.rs b/src/libsyntax/parse/attr.rs index a0defbc09dc04..9eac024edb17e 100644 --- a/src/libsyntax/parse/attr.rs +++ b/src/libsyntax/parse/attr.rs @@ -125,8 +125,8 @@ impl<'a> Parser<'a> { self.expect(&token::OpenDelim(token::Bracket))?; let meta_item = self.parse_meta_item()?; - let hi = self.last_span.hi; self.expect(&token::CloseDelim(token::Bracket))?; + let hi = self.last_span.hi; (mk_sp(lo, hi), meta_item, style) } diff --git a/src/test/ui/span/issue-36530.rs b/src/test/ui/span/issue-36530.rs new file mode 100644 index 0000000000000..893c2168c2e16 --- /dev/null +++ b/src/test/ui/span/issue-36530.rs @@ -0,0 +1,14 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[foo] +mod foo { + #![foo] +} diff --git a/src/test/ui/span/issue-36530.stderr b/src/test/ui/span/issue-36530.stderr new file mode 100644 index 0000000000000..dc6190c2e76b0 --- /dev/null +++ b/src/test/ui/span/issue-36530.stderr @@ -0,0 +1,18 @@ +error: The attribute `foo` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642) + --> $DIR/issue-36530.rs:11:1 + | +11 | #[foo] + | ^^^^^^ + | + = help: add #![feature(custom_attribute)] to the crate attributes to enable + +error: The attribute `foo` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642) + --> $DIR/issue-36530.rs:13:5 + | +13 | #![foo] + | ^^^^^^^ + | + = help: add #![feature(custom_attribute)] to the crate attributes to enable + +error: aborting due to 2 previous errors + From d8b2cfeae6fbc1a9d7e86c9809f27ad1200903cb Mon Sep 17 00:00:00 2001 From: Cobrand Date: Fri, 16 Sep 2016 23:52:03 +0200 Subject: [PATCH 5/5] Remove stray println! when invoking error E0316 --- src/librustc/middle/resolve_lifetime.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 95706b5677a62..2d93c33afb409 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -337,7 +337,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for LifetimeContext<'a, 'tcx> { if !self.trait_ref_hack || !trait_ref.bound_lifetimes.is_empty() { if self.trait_ref_hack { - println!("{:?}", trait_ref.span); span_err!(self.sess, trait_ref.span, E0316, "nested quantification of lifetimes"); }