Add a target option "merge-functions" taking values in ("disabled",

"trampolines", or "aliases (the default)) to allow targets to opt out of
the MergeFunctions LLVM pass. Also add a corresponding -Z option with
the same name and values.

This works around: https://github.com/rust-lang/rust/issues/57356

Motivation:

Basically, the problem is that the MergeFunctions pass, which rustc
currently enables by default at -O2 and -O3, and `extern "ptx-kernel"`
functions (specific to the NVPTX target) are currently not compatible
with each other. If the MergeFunctions pass is allowed to run, rustc can
generate invalid PTX assembly (i.e. a PTX file that is not accepted by
the native PTX assembler ptxas). Therefore we would like a way to opt
out of the MergeFunctions pass, which is what our target option does.

Related work:

The current behavior of rustc is to enable MergeFunctions at -O2 and -O3,
and also to enable the use of function aliases within MergeFunctions.
MergeFunctions both with and without function aliases is incompatible with
the NVPTX target.

clang's "solution" is to have a "-fmerge-functions" flag that opts in to
the MergeFunctions pass, but it is not enabled by default.
This commit is contained in:
Peter Jin 2018-12-31 10:58:13 -08:00
parent 2442823ef5
commit b91d211b40
4 changed files with 117 additions and 8 deletions

View file

@ -6,7 +6,7 @@ use std::str::FromStr;
use session::{early_error, early_warn, Session}; use session::{early_error, early_warn, Session};
use session::search_paths::SearchPath; use session::search_paths::SearchPath;
use rustc_target::spec::{LinkerFlavor, PanicStrategy, RelroLevel}; use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelroLevel};
use rustc_target::spec::{Target, TargetTriple}; use rustc_target::spec::{Target, TargetTriple};
use lint; use lint;
use middle::cstore; use middle::cstore;
@ -808,13 +808,16 @@ macro_rules! options {
pub const parse_cross_lang_lto: Option<&str> = pub const parse_cross_lang_lto: Option<&str> =
Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \ Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \
or the path to the linker plugin"); or the path to the linker plugin");
pub const parse_merge_functions: Option<&str> =
Some("one of: `disabled`, `trampolines`, or `aliases`");
} }
#[allow(dead_code)] #[allow(dead_code)]
mod $mod_set { mod $mod_set {
use super::{$struct_name, Passes, Sanitizer, LtoCli, CrossLangLto}; use super::{$struct_name, Passes, Sanitizer, LtoCli, CrossLangLto};
use rustc_target::spec::{LinkerFlavor, PanicStrategy, RelroLevel}; use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelroLevel};
use std::path::PathBuf; use std::path::PathBuf;
use std::str::FromStr;
$( $(
pub fn $opt(cg: &mut $struct_name, v: Option<&str>) -> bool { pub fn $opt(cg: &mut $struct_name, v: Option<&str>) -> bool {
@ -1046,6 +1049,14 @@ macro_rules! options {
}; };
true true
} }
fn parse_merge_functions(slot: &mut Option<MergeFunctions>, v: Option<&str>) -> bool {
match v.and_then(|s| MergeFunctions::from_str(s).ok()) {
Some(mergefunc) => *slot = Some(mergefunc),
_ => return false,
}
true
}
} }
) } ) }
@ -1380,6 +1391,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"whether to use the PLT when calling into shared libraries; "whether to use the PLT when calling into shared libraries;
only has effect for PIC code on systems with ELF binaries only has effect for PIC code on systems with ELF binaries
(default: PLT is disabled if full relro is enabled)"), (default: PLT is disabled if full relro is enabled)"),
merge_functions: Option<MergeFunctions> = (None, parse_merge_functions, [TRACKED],
"control the operation of the MergeFunctions LLVM pass, taking
the same values as the target option of the same name"),
} }
pub fn default_lib_output() -> CrateType { pub fn default_lib_output() -> CrateType {
@ -2398,7 +2412,7 @@ mod dep_tracking {
use super::{CrateType, DebugInfo, ErrorOutputType, OptLevel, OutputTypes, use super::{CrateType, DebugInfo, ErrorOutputType, OptLevel, OutputTypes,
Passes, Sanitizer, LtoCli, CrossLangLto}; Passes, Sanitizer, LtoCli, CrossLangLto};
use syntax::feature_gate::UnstableFeatures; use syntax::feature_gate::UnstableFeatures;
use rustc_target::spec::{PanicStrategy, RelroLevel, TargetTriple}; use rustc_target::spec::{MergeFunctions, PanicStrategy, RelroLevel, TargetTriple};
use syntax::edition::Edition; use syntax::edition::Edition;
pub trait DepTrackingHash { pub trait DepTrackingHash {
@ -2441,12 +2455,14 @@ mod dep_tracking {
impl_dep_tracking_hash_via_hash!(Option<usize>); impl_dep_tracking_hash_via_hash!(Option<usize>);
impl_dep_tracking_hash_via_hash!(Option<String>); impl_dep_tracking_hash_via_hash!(Option<String>);
impl_dep_tracking_hash_via_hash!(Option<(String, u64)>); impl_dep_tracking_hash_via_hash!(Option<(String, u64)>);
impl_dep_tracking_hash_via_hash!(Option<MergeFunctions>);
impl_dep_tracking_hash_via_hash!(Option<PanicStrategy>); impl_dep_tracking_hash_via_hash!(Option<PanicStrategy>);
impl_dep_tracking_hash_via_hash!(Option<RelroLevel>); impl_dep_tracking_hash_via_hash!(Option<RelroLevel>);
impl_dep_tracking_hash_via_hash!(Option<lint::Level>); impl_dep_tracking_hash_via_hash!(Option<lint::Level>);
impl_dep_tracking_hash_via_hash!(Option<PathBuf>); impl_dep_tracking_hash_via_hash!(Option<PathBuf>);
impl_dep_tracking_hash_via_hash!(Option<cstore::NativeLibraryKind>); impl_dep_tracking_hash_via_hash!(Option<cstore::NativeLibraryKind>);
impl_dep_tracking_hash_via_hash!(CrateType); impl_dep_tracking_hash_via_hash!(CrateType);
impl_dep_tracking_hash_via_hash!(MergeFunctions);
impl_dep_tracking_hash_via_hash!(PanicStrategy); impl_dep_tracking_hash_via_hash!(PanicStrategy);
impl_dep_tracking_hash_via_hash!(RelroLevel); impl_dep_tracking_hash_via_hash!(RelroLevel);
impl_dep_tracking_hash_via_hash!(Passes); impl_dep_tracking_hash_via_hash!(Passes);
@ -2532,7 +2548,7 @@ mod tests {
use std::iter::FromIterator; use std::iter::FromIterator;
use std::path::PathBuf; use std::path::PathBuf;
use super::{Externs, OutputType, OutputTypes}; use super::{Externs, OutputType, OutputTypes};
use rustc_target::spec::{PanicStrategy, RelroLevel}; use rustc_target::spec::{MergeFunctions, PanicStrategy, RelroLevel};
use syntax::symbol::Symbol; use syntax::symbol::Symbol;
use syntax::edition::{Edition, DEFAULT_EDITION}; use syntax::edition::{Edition, DEFAULT_EDITION};
use syntax; use syntax;
@ -3187,6 +3203,10 @@ mod tests {
opts = reference.clone(); opts = reference.clone();
opts.debugging_opts.cross_lang_lto = CrossLangLto::LinkerPluginAuto; opts.debugging_opts.cross_lang_lto = CrossLangLto::LinkerPluginAuto;
assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash());
opts = reference.clone();
opts.debugging_opts.merge_functions = Some(MergeFunctions::Disabled);
assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash());
} }
#[test] #[test]

View file

@ -3,6 +3,7 @@ use back::write::create_target_machine;
use llvm; use llvm;
use rustc::session::Session; use rustc::session::Session;
use rustc::session::config::PrintRequest; use rustc::session::config::PrintRequest;
use rustc_target::spec::MergeFunctions;
use libc::c_int; use libc::c_int;
use std::ffi::CString; use std::ffi::CString;
use syntax::feature_gate::UnstableFeatures; use syntax::feature_gate::UnstableFeatures;
@ -61,8 +62,15 @@ unsafe fn configure_llvm(sess: &Session) {
add("-disable-preinline"); add("-disable-preinline");
} }
if llvm::LLVMRustIsRustLLVM() { if llvm::LLVMRustIsRustLLVM() {
match sess.opts.debugging_opts.merge_functions
.unwrap_or(sess.target.target.options.merge_functions) {
MergeFunctions::Disabled |
MergeFunctions::Trampolines => {}
MergeFunctions::Aliases => {
add("-mergefunc-use-aliases"); add("-mergefunc-use-aliases");
} }
}
}
// HACK(eddyb) LLVM inserts `llvm.assume` calls to preserve align attributes // HACK(eddyb) LLVM inserts `llvm.assume` calls to preserve align attributes
// during inlining. Unfortunately these may block other optimizations. // during inlining. Unfortunately these may block other optimizations.

View file

@ -24,6 +24,7 @@ use rustc_fs_util::link_or_copy;
use rustc_data_structures::svh::Svh; use rustc_data_structures::svh::Svh;
use rustc_errors::{Handler, Level, DiagnosticBuilder, FatalError, DiagnosticId}; use rustc_errors::{Handler, Level, DiagnosticBuilder, FatalError, DiagnosticId};
use rustc_errors::emitter::{Emitter}; use rustc_errors::emitter::{Emitter};
use rustc_target::spec::MergeFunctions;
use syntax::attr; use syntax::attr;
use syntax::ext::hygiene::Mark; use syntax::ext::hygiene::Mark;
use syntax_pos::MultiSpan; use syntax_pos::MultiSpan;
@ -152,8 +153,24 @@ impl ModuleConfig {
sess.opts.optimize == config::OptLevel::Aggressive && sess.opts.optimize == config::OptLevel::Aggressive &&
!sess.target.target.options.is_like_emscripten; !sess.target.target.options.is_like_emscripten;
self.merge_functions = sess.opts.optimize == config::OptLevel::Default || // Some targets (namely, NVPTX) interact badly with the MergeFunctions
sess.opts.optimize == config::OptLevel::Aggressive; // pass. This is because MergeFunctions can generate new function calls
// which may interfere with the target calling convention; e.g. for the
// NVPTX target, PTX kernels should not call other PTX kernels.
// MergeFunctions can also be configured to generate aliases instead,
// but aliases are not supported by some backends (again, NVPTX).
// Therefore, allow targets to opt out of the MergeFunctions pass,
// but otherwise keep the pass enabled (at O2 and O3) since it can be
// useful for reducing code size.
self.merge_functions = match sess.opts.debugging_opts.merge_functions
.unwrap_or(sess.target.target.options.merge_functions) {
MergeFunctions::Disabled => false,
MergeFunctions::Trampolines |
MergeFunctions::Aliases => {
sess.opts.optimize == config::OptLevel::Default ||
sess.opts.optimize == config::OptLevel::Aggressive
}
};
} }
pub fn bitcode_needed(&self) -> bool { pub fn bitcode_needed(&self) -> bool {

View file

@ -217,6 +217,46 @@ impl ToJson for RelroLevel {
} }
} }
#[derive(Clone, Copy, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)]
pub enum MergeFunctions {
Disabled,
Trampolines,
Aliases
}
impl MergeFunctions {
pub fn desc(&self) -> &str {
match *self {
MergeFunctions::Disabled => "disabled",
MergeFunctions::Trampolines => "trampolines",
MergeFunctions::Aliases => "aliases",
}
}
}
impl FromStr for MergeFunctions {
type Err = ();
fn from_str(s: &str) -> Result<MergeFunctions, ()> {
match s {
"disabled" => Ok(MergeFunctions::Disabled),
"trampolines" => Ok(MergeFunctions::Trampolines),
"aliases" => Ok(MergeFunctions::Aliases),
_ => Err(()),
}
}
}
impl ToJson for MergeFunctions {
fn to_json(&self) -> Json {
match *self {
MergeFunctions::Disabled => "disabled".to_json(),
MergeFunctions::Trampolines => "trampolines".to_json(),
MergeFunctions::Aliases => "aliases".to_json(),
}
}
}
pub type LinkArgs = BTreeMap<LinkerFlavor, Vec<String>>; pub type LinkArgs = BTreeMap<LinkerFlavor, Vec<String>>;
pub type TargetResult = Result<Target, String>; pub type TargetResult = Result<Target, String>;
@ -690,7 +730,15 @@ pub struct TargetOptions {
/// If set, have the linker export exactly these symbols, instead of using /// If set, have the linker export exactly these symbols, instead of using
/// the usual logic to figure this out from the crate itself. /// the usual logic to figure this out from the crate itself.
pub override_export_symbols: Option<Vec<String>> pub override_export_symbols: Option<Vec<String>>,
/// Determines how or whether the MergeFunctions LLVM pass should run for
/// this target. Either "disabled", "trampolines", or "aliases".
/// The MergeFunctions pass is generally useful, but some targets may need
/// to opt out. The default is "aliases".
///
/// Workaround for: https://github.com/rust-lang/rust/issues/57356
pub merge_functions: MergeFunctions
} }
impl Default for TargetOptions { impl Default for TargetOptions {
@ -773,6 +821,7 @@ impl Default for TargetOptions {
requires_uwtable: false, requires_uwtable: false,
simd_types_indirect: true, simd_types_indirect: true,
override_export_symbols: None, override_export_symbols: None,
merge_functions: MergeFunctions::Aliases,
} }
} }
} }
@ -875,6 +924,19 @@ impl Target {
.map(|o| o.as_u64() .map(|o| o.as_u64()
.map(|s| base.options.$key_name = Some(s))); .map(|s| base.options.$key_name = Some(s)));
} ); } );
($key_name:ident, MergeFunctions) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<MergeFunctions>() {
Ok(mergefunc) => base.options.$key_name = mergefunc,
_ => return Some(Err(format!("'{}' is not a valid value for \
merge-functions. Use 'disabled', \
'trampolines', or 'aliases'.",
s))),
}
Some(Ok(()))
})).unwrap_or(Ok(()))
} );
($key_name:ident, PanicStrategy) => ( { ($key_name:ident, PanicStrategy) => ( {
let name = (stringify!($key_name)).replace("_", "-"); let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| { obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
@ -1064,6 +1126,7 @@ impl Target {
key!(requires_uwtable, bool); key!(requires_uwtable, bool);
key!(simd_types_indirect, bool); key!(simd_types_indirect, bool);
key!(override_export_symbols, opt_list); key!(override_export_symbols, opt_list);
key!(merge_functions, MergeFunctions)?;
if let Some(array) = obj.find("abi-blacklist").and_then(Json::as_array) { if let Some(array) = obj.find("abi-blacklist").and_then(Json::as_array) {
for name in array.iter().filter_map(|abi| abi.as_string()) { for name in array.iter().filter_map(|abi| abi.as_string()) {
@ -1275,6 +1338,7 @@ impl ToJson for Target {
target_option_val!(requires_uwtable); target_option_val!(requires_uwtable);
target_option_val!(simd_types_indirect); target_option_val!(simd_types_indirect);
target_option_val!(override_export_symbols); target_option_val!(override_export_symbols);
target_option_val!(merge_functions);
if default.abi_blacklist != self.options.abi_blacklist { if default.abi_blacklist != self.options.abi_blacklist {
d.insert("abi-blacklist".to_string(), self.options.abi_blacklist.iter() d.insert("abi-blacklist".to_string(), self.options.abi_blacklist.iter()