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

Implement optimization fuel and re-enable struct field reordering #40377

Merged
merged 9 commits into from
Apr 12, 2017
24 changes: 24 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,8 @@ macro_rules! options {
Some("one of: `address`, `leak`, `memory` or `thread`");
pub const parse_linker_flavor: Option<&'static str> =
Some(::rustc_back::LinkerFlavor::one_of());
pub const parse_optimization_fuel: Option<&'static str> =
Some("crate=integer");
}

#[allow(dead_code)]
Expand Down Expand Up @@ -787,6 +789,21 @@ macro_rules! options {
}
true
}

fn parse_optimization_fuel(slot: &mut Option<(String, u64)>, v: Option<&str>) -> bool {
match v {
None => false,
Some(s) => {
let parts = s.split('=').collect::<Vec<_>>();
if parts.len() != 2 { return false; }
let crate_name = parts[0].to_string();
let fuel = parts[1].parse::<u64>();
if fuel.is_err() { return false; }
*slot = Some((crate_name, fuel.unwrap()));
true
}
}
}
}
) }

Expand Down Expand Up @@ -991,6 +1008,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"Use a sanitizer"),
linker_flavor: Option<LinkerFlavor> = (None, parse_linker_flavor, [UNTRACKED],
"Linker flavor"),
fuel: Option<(String, u64)> = (None, parse_optimization_fuel, [TRACKED],
"Set the optimization fuel quota for a crate."),
print_fuel: Option<String> = (None, parse_opt_string, [TRACKED],
"Make Rustc print the total optimization fuel used by a crate."),
}

pub fn default_lib_output() -> CrateType {
Expand Down Expand Up @@ -1784,11 +1805,13 @@ mod dep_tracking {

impl_dep_tracking_hash_via_hash!(bool);
impl_dep_tracking_hash_via_hash!(usize);
impl_dep_tracking_hash_via_hash!(u64);
impl_dep_tracking_hash_via_hash!(String);
impl_dep_tracking_hash_via_hash!(lint::Level);
impl_dep_tracking_hash_via_hash!(Option<bool>);
impl_dep_tracking_hash_via_hash!(Option<usize>);
impl_dep_tracking_hash_via_hash!(Option<String>);
impl_dep_tracking_hash_via_hash!(Option<(String, u64)>);
impl_dep_tracking_hash_via_hash!(Option<PanicStrategy>);
impl_dep_tracking_hash_via_hash!(Option<lint::Level>);
impl_dep_tracking_hash_via_hash!(Option<PathBuf>);
Expand All @@ -1810,6 +1833,7 @@ mod dep_tracking {
impl_dep_tracking_hash_for_sortable_vec_of!((String, lint::Level));
impl_dep_tracking_hash_for_sortable_vec_of!((String, Option<String>,
Option<cstore::NativeLibraryKind>));
impl_dep_tracking_hash_for_sortable_vec_of!((String, u64));
impl DepTrackingHash for SearchPaths {
fn hash(&self, hasher: &mut DefaultHasher, _: ErrorOutputType) {
let mut elems: Vec<_> = self
Expand Down
51 changes: 51 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,20 @@ pub struct Session {
pub code_stats: RefCell<CodeStats>,

next_node_id: Cell<ast::NodeId>,

/// If -zfuel=crate=n is specified, Some(crate).
optimization_fuel_crate: Option<String>,
/// If -zfuel=crate=n is specified, initially set to n. Otherwise 0.
optimization_fuel_limit: Cell<u64>,
/// We're rejecting all further optimizations.
out_of_fuel: Cell<bool>,

// The next two are public because the driver needs to read them.

/// If -zprint-fuel=crate, Some(crate).
pub print_fuel_crate: Option<String>,
/// Always set to zero and incremented so that we can print fuel expended by a crate.
pub print_fuel: Cell<u64>,
}

pub struct PerfStats {
Expand Down Expand Up @@ -507,6 +521,32 @@ impl Session {
println!("Total time spent decoding DefPath tables: {}",
duration_to_secs_str(self.perf_stats.decode_def_path_tables_time.get()));
}

/// We want to know if we're allowed to do an optimization for crate foo from -z fuel=foo=n.
/// This expends fuel if applicable, and records fuel if applicable.
pub fn consider_optimizing<T: Fn() -> String>(&self, crate_name: &str, msg: T) -> bool {
let mut ret = true;
match self.optimization_fuel_crate {
Some(ref c) if c == crate_name => {
let fuel = self.optimization_fuel_limit.get();
ret = fuel != 0;
if fuel == 0 && !self.out_of_fuel.get() {
println!("optimization-fuel-exhausted: {}", msg());
self.out_of_fuel.set(true);
} else if fuel > 0 {
self.optimization_fuel_limit.set(fuel-1);
}
}
_ => {}
}
match self.print_fuel_crate {
Some(ref c) if c == crate_name=> {
self.print_fuel.set(self.print_fuel.get()+1);
},
_ => {}
}
ret
}
}

pub fn build_session(sopts: config::Options,
Expand Down Expand Up @@ -602,6 +642,12 @@ pub fn build_session_(sopts: config::Options,
}
);

let optimization_fuel_crate = sopts.debugging_opts.fuel.as_ref().map(|i| i.0.clone());
let optimization_fuel_limit = Cell::new(sopts.debugging_opts.fuel.as_ref()
.map(|i| i.1).unwrap_or(0));
let print_fuel_crate = sopts.debugging_opts.print_fuel.clone();
let print_fuel = Cell::new(0);

let sess = Session {
dep_graph: dep_graph.clone(),
target: target_cfg,
Expand Down Expand Up @@ -643,6 +689,11 @@ pub fn build_session_(sopts: config::Options,
decode_def_path_tables_time: Cell::new(Duration::from_secs(0)),
},
code_stats: RefCell::new(CodeStats::new()),
optimization_fuel_crate: optimization_fuel_crate,
optimization_fuel_limit: optimization_fuel_limit,
print_fuel_crate: print_fuel_crate,
print_fuel: print_fuel,
out_of_fuel: Cell::new(false),
};

init_llvm(&sess);
Expand Down
5 changes: 5 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
ast_ty_to_ty_cache: RefCell::new(NodeMap()),
}, f)
}

pub fn consider_optimizing<T: Fn() -> String>(&self, msg: T) -> bool {
let cname = self.crate_name(LOCAL_CRATE).as_str();
self.sess.consider_optimizing(&cname, msg)
}
}

impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
Expand Down
9 changes: 2 additions & 7 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,6 @@ enum StructKind {
}

impl<'a, 'gcx, 'tcx> Struct {
// FIXME(camlorn): reprs need a better representation to deal with multiple reprs on one type.
fn new(dl: &TargetDataLayout, fields: &Vec<&'a Layout>,
repr: &ReprOptions, kind: StructKind,
scapegoat: Ty<'gcx>) -> Result<Struct, LayoutError<'gcx>> {
Expand All @@ -598,12 +597,8 @@ impl<'a, 'gcx, 'tcx> Struct {
// Neither do 1-member and 2-member structs.
// In addition, code in trans assume that 2-element structs can become pairs.
// It's easier to just short-circuit here.
let mut can_optimize = (fields.len() > 2 || StructKind::EnumVariant == kind)
&& ! (repr.c || repr.packed);

// Disable field reordering until we can decide what to do.
// The odd pattern here avoids a warning about the value never being read.
if can_optimize { can_optimize = false; }
let can_optimize = (fields.len() > 2 || StructKind::EnumVariant == kind)
&& !(repr.c || repr.packed || repr.linear || repr.simd);

let (optimize, sort_ascending) = match kind {
StructKind::AlwaysSizedUnivariant => (can_optimize, false),
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,13 +1411,16 @@ pub struct ReprOptions {
pub packed: bool,
pub simd: bool,
pub int: Option<attr::IntType>,
// Internal only for now. If true, don't reorder fields.
pub linear: bool,
}

impl_stable_hash_for!(struct ReprOptions {
c,
packed,
simd,
int
int,
linear
});

impl ReprOptions {
Expand All @@ -1440,6 +1443,9 @@ impl ReprOptions {
ret.simd = true;
}

// This is here instead of layout because the choice must make it into metadata.
ret.linear = !tcx.consider_optimizing(|| format!("Reorder fields of {:?}",
tcx.item_path_str(did)));
ret
}

Expand Down
10 changes: 10 additions & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,16 @@ impl<'a> CompilerCalls<'a> for RustcDefaultCalls {
control.make_glob_map = resolve::MakeGlobMap::Yes;
}

if sess.print_fuel_crate.is_some() {
let old_callback = control.compilation_done.callback;
control.compilation_done.callback = box move |state| {
old_callback(state);
let sess = state.session;
println!("Fuel used by {}: {}",
sess.print_fuel_crate.as_ref().unwrap(),
sess.print_fuel.get());
}
}
control
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_trans/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use llvm;
use llvm::{ValueRef};
use abi::{Abi, FnType};
use adt;
use mir::lvalue::LvalueRef;
use mir::lvalue::{LvalueRef, Alignment};
use base::*;
use common::*;
use declare;
Expand All @@ -36,8 +36,6 @@ use syntax_pos::Span;
use std::cmp::Ordering;
use std::iter;

use mir::lvalue::Alignment;

fn get_simple_intrinsic(ccx: &CrateContext, name: &str) -> Option<ValueRef> {
let llvm_name = match name {
"sqrtf32" => "llvm.sqrt.f32",
Expand Down Expand Up @@ -622,7 +620,10 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,

for i in 0..elems.len() {
let val = bcx.extract_value(val, i);
bcx.store(val, bcx.struct_gep(llresult, i), None);
let lval = LvalueRef::new_sized_ty(llresult, ret_ty,
Alignment::AbiAligned);
let (dest, align) = lval.trans_field_ptr(bcx, i);
bcx.store(val, dest, align.to_align());
}
C_nil(ccx)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ fn arg_local_refs<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,

let lvalue = LvalueRef::alloca(bcx, arg_ty, &format!("arg{}", arg_index));
for (i, &tupled_arg_ty) in tupled_arg_tys.iter().enumerate() {
let dst = bcx.struct_gep(lvalue.llval, i);
let (dst, _) = lvalue.trans_field_ptr(bcx, i);
Copy link
Member

Choose a reason for hiding this comment

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

Same but here it's less worrying because we know it can't be anything other than a tuple, and those don't get packed or anything like that, and store_fn_arg doesn't take such an argument.

let arg = &mircx.fn_ty.args[idx];
idx += 1;
if common::type_is_fat_ptr(bcx.ccx, tupled_arg_ty) {
Expand Down
24 changes: 24 additions & 0 deletions src/test/run-pass/optimization-fuel-0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2012 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.

#![crate_name="foo"]

use std::mem::size_of;

// compile-flags: -Z fuel=foo=0

struct S1(u8, u16, u8);
struct S2(u8, u16, u8);

fn main() {
assert_eq!(size_of::<S1>(), 6);
assert_eq!(size_of::<S2>(), 6);
}

26 changes: 26 additions & 0 deletions src/test/run-pass/optimization-fuel-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2012 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.

#![crate_name="foo"]

use std::mem::size_of;

// compile-flags: -Z fuel=foo=1

struct S1(u8, u16, u8);
struct S2(u8, u16, u8);

fn main() {
let optimized = (size_of::<S1>() == 4) as usize
+(size_of::<S2>() == 4) as usize;
assert_eq!(optimized, 1);
}


13 changes: 13 additions & 0 deletions src/test/run-pass/type-sizes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ enum e3 {
a([u16; 0], u8), b
}

struct ReorderedStruct {
a: u8,
b: u16,
c: u8
}

enum ReorderedEnum {
A(u8, u16, u8),
B(u8, u16, u8),
}

pub fn main() {
assert_eq!(size_of::<u8>(), 1 as usize);
assert_eq!(size_of::<u32>(), 4 as usize);
Expand All @@ -54,4 +65,6 @@ pub fn main() {
assert_eq!(size_of::<e1>(), 8 as usize);
assert_eq!(size_of::<e2>(), 8 as usize);
assert_eq!(size_of::<e3>(), 4 as usize);
assert_eq!(size_of::<ReorderedStruct>(), 4);
assert_eq!(size_of::<ReorderedEnum>(), 6);
}
21 changes: 21 additions & 0 deletions src/test/ui/print-fuel/print-fuel.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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 <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.

#![crate_name="foo"]
#![allow(dead_code)]

// compile-flags: -Z print-fuel=foo

struct S1(u8, u16, u8);
struct S2(u8, u16, u8);
struct S3(u8, u16, u8);

fn main() {
}
1 change: 1 addition & 0 deletions src/test/ui/print-fuel/print-fuel.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fuel used by foo: 3
Loading