Skip to content

Commit

Permalink
Rollup merge of #40377 - camlorn:optimization_fuel, r=eddyb
Browse files Browse the repository at this point in the history
Implement optimization fuel and re-enable struct field reordering

See [this discussion](https://internals.rust-lang.org/t/rolling-out-or-unrolling-struct-field-reorderings/4485) for background.

This pull request adds two new compilation options: `-Z print-fuel=crate` prints the optimization fuel used by a crate and `-Z fuel=crate=n` sets the optimization fuel for a crate.

It also turns field reordering back on.  There is no way to test this feature without something consuming fuel.  We can roll this back if we want, but then the optimization fuel bits will be dead code.

The one notable absence from this PR is a test case.  I'm not sure how to do one that's worth having.  The only thing I can think of to test is `-Z fuel=foo=0`.  The problem with other tests is that either (1) they're so big that future optimizations will apply, thus breaking them or (2) we don't know which order the optimizations will be applied in, so we can't guess the message that will be printed.  If someone has a useful proposal for a good test, I certainly want to add one.
  • Loading branch information
frewsxcv authored Apr 11, 2017
2 parents c58c928 + e18c59f commit 4f6f4eb
Show file tree
Hide file tree
Showing 16 changed files with 219 additions and 45 deletions.
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);
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

0 comments on commit 4f6f4eb

Please sign in to comment.