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

Compute target_feature from LLVM #31709

Merged
merged 10 commits into from
Apr 20, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mk/crates.mk
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ DEPS_rustc_const_eval := rustc_const_math rustc syntax log serialize \
rustc_back graphviz

DEPS_rustc := syntax fmt_macros flate arena serialize getopts rbml \
log graphviz rustc_back rustc_data_structures\
log graphviz rustc_llvm rustc_back rustc_data_structures\
rustc_const_math
DEPS_rustc_back := std syntax flate log libc
DEPS_rustc_borrowck := rustc rustc_mir log graphviz syntax
Expand Down
4 changes: 4 additions & 0 deletions mk/rustllvm.mk
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ $$(RT_OUTPUT_DIR_$(1))/$$(call CFG_STATIC_LIB_NAME_$(1),rustllvm): \
@$$(call E, link: $$@)
$$(Q)$$(call CFG_CREATE_ARCHIVE_$(1),$$@) $$^

RUSTLLVM_COMPONENTS_$(1) = $$(shell echo $$(LLVM_ALL_COMPONENTS_$(1)) |\
tr 'a-z-' 'A-Z_'| sed -e 's/^ //;s/\([^ ]*\)/\-DLLVM_COMPONENT_\1/g')

# On MSVC we need to double-escape arguments that llvm-config printed which
# start with a '/'. The shell we're running in will auto-translate the argument
# `/foo` to `C:/msys64/foo` but we really want it to be passed through as `/foo`
Expand All @@ -51,6 +54,7 @@ $(1)/rustllvm/%.o: $(S)src/rustllvm/%.cpp $$(MKFILE_DEPS) $$(LLVM_CONFIG_$(1))
@$$(call E, compile: $$@)
$$(Q)$$(call CFG_COMPILE_CXX_$(1), $$@,) \
$$(subst /,//,$$(LLVM_CXXFLAGS_$(1))) \
$$(RUSTLLVM_COMPONENTS_$(1)) \
$$(EXTRA_RUSTLLVM_CXXFLAGS_$(1)) \
$$(RUSTLLVM_INCS_$(1)) \
$$<
Expand Down
1 change: 1 addition & 0 deletions src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ rustc_back = { path = "../librustc_back" }
rustc_bitflags = { path = "../librustc_bitflags" }
rustc_const_math = { path = "../librustc_const_math" }
rustc_data_structures = { path = "../librustc_data_structures" }
rustc_llvm = { path = "../librustc_llvm" }
serialize = { path = "../libserialize" }
syntax = { path = "../libsyntax" }
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ extern crate getopts;
extern crate graphviz;
extern crate libc;
extern crate rbml;
extern crate rustc_llvm as llvm;
extern crate rustc_back;
extern crate rustc_data_structures;
extern crate serialize;
Expand Down
57 changes: 57 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ use syntax::{ast, codemap};
use syntax::feature_gate::AttributeType;

use rustc_back::target::Target;
use llvm;

use std::path::{Path, PathBuf};
use std::cell::{Cell, RefCell};
use std::collections::{HashMap, HashSet};
use std::env;
use std::ffi::CString;
use std::rc::Rc;
use std::fmt;
use libc::c_int;

pub mod config;
pub mod filesearch;
Expand Down Expand Up @@ -491,9 +494,63 @@ pub fn build_session_(sopts: config::Options,
imported_macro_spans: RefCell::new(HashMap::new()),
};

init_llvm(&sess);

sess
}

fn init_llvm(sess: &Session) {
unsafe {
// Before we touch LLVM, make sure that multithreading is enabled.
use std::sync::Once;
static INIT: Once = Once::new();
static mut POISONED: bool = false;
INIT.call_once(|| {
if llvm::LLVMStartMultithreaded() != 1 {
// use an extra bool to make sure that all future usage of LLVM
// cannot proceed despite the Once not running more than once.
POISONED = true;
}

configure_llvm(sess);
});

if POISONED {
bug!("couldn't enable multi-threaded LLVM");
}
}
}

unsafe fn configure_llvm(sess: &Session) {
let mut llvm_c_strs = Vec::new();
let mut llvm_args = Vec::new();

{
let mut add = |arg: &str| {
let s = CString::new(arg).unwrap();
llvm_args.push(s.as_ptr());
llvm_c_strs.push(s);
};
add("rustc"); // fake program name
if sess.time_llvm_passes() { add("-time-passes"); }
if sess.print_llvm_passes() { add("-debug-pass=Structure"); }

// FIXME #21627 disable faulty FastISel on AArch64 (even for -O0)
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is safe to remove now; #21627 references an upstream fix which went into llvm 18 months ago llvm-mirror/llvm@ca07e25

Copy link
Member

Choose a reason for hiding this comment

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

Awesome! Would you be interested in prep'ing a separate PR for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure. I'll open a new PR if that removal doesn't happen here.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it's probably good to keep these separate, so let's split that change out of this PR.

if sess.target.target.arch == "aarch64" { add("-fast-isel=0"); }

for arg in &sess.opts.cg.llvm_args {
add(&(*arg));
}
}

llvm::LLVMInitializePasses();

llvm::initialize_available_targets();

llvm::LLVMRustSetLLVMOptions(llvm_args.len() as c_int,
llvm_args.as_ptr());
}

pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
let mut emitter: Box<Emitter> = match output {
config::ErrorOutputType::HumanReadable(color_config) => {
Expand Down
105 changes: 40 additions & 65 deletions src/librustc_driver/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,79 +9,54 @@
// except according to those terms.

use syntax::{ast, attr};
use llvm::LLVMRustHasFeature;
use rustc::session::Session;
use rustc_trans::back::write::create_target_machine;
use syntax::parse::token::InternedString;
use syntax::parse::token::intern_and_get_ident as intern;
use libc::c_char;

// WARNING: the features must be known to LLVM or the feature
// detection code will walk past the end of the feature array,
// leading to crashes.

const ARM_WHITELIST: &'static [&'static str] = &[
"neon\0",
"vfp2\0",
"vfp3\0",
"vfp4\0",
];

const X86_WHITELIST: &'static [&'static str] = &[
"avx\0",
"avx2\0",
"sse\0",
"sse2\0",
"sse3\0",
"sse4.1\0",
"sse4.2\0",
"ssse3\0",
];

/// Add `target_feature = "..."` cfgs for a variety of platform
/// specific features (SSE, NEON etc.).
///
/// This uses a scheme similar to that employed by clang: reimplement
/// the target feature knowledge. *Theoretically* we could query LLVM
/// since that has perfect knowledge about what things are enabled in
/// code-generation, however, it is extremely non-obvious how to do
/// this successfully. Each platform defines a subclass of a
/// SubtargetInfo, which knows all this information, but the ways to
/// query them do not seem to be public.
/// This is performed by checking whether a whitelisted set of
/// features is available on the target machine, by querying LLVM.
Copy link
Member

Choose a reason for hiding this comment

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

One of the purposes of a macro like this was to ensure that we had control over the names exported. We don't want to blanket export all features that LLVM itself exposes, and we also don't want to tie ourselves to exactly the names LLVM has. In other words this is here to ensure we can provide a stable interface over the possibly unstable interface of LLVM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filtering and renaming these features to a "rust" convention instead of the LLVM one can be done very easily with a map. The filtering makes sense to me, as rust might want to expose only a subset of the LLVM features through cfg!.
Renaming these features is just as easy, but I would be surprised if it happened only here and not in every place where a feature is named (for example when they are passed through the -C target-feature flag). That would result in an inconsistent naming, as the same feature would have a different name in the command line flags and in the code.
If you confirm that renaming is desired here and not elsewhere, I will implement it, otherwise I believe that it might belong to another PR (under the assumption that if LLVM and rust feature naming was to diverge in future, we would decide how to take care of that and possibly perform a consistent renaming everywhere).

Copy link
Member

Choose a reason for hiding this comment

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

The purpose of this macro was that for the function/name pair the cfg directive name would be defined if the function returned true. This is the only place that does that.

Basically, we need to filter what's coming out of LLVM with an explicit whitelist of what cfg directives are defined. We don't need to map names yet (as we just use LLVM names), but we may support that one day.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a whitelist of known LLVM features which is used to filter which ones are exposed through cfg!.

pub fn add_configuration(cfg: &mut ast::CrateConfig, sess: &Session) {
let tf = InternedString::new("target_feature");
macro_rules! fillout {
($($func: ident, $name: expr;)*) => {{
$(if $func(sess) {
cfg.push(attr::mk_name_value_item_str(tf.clone(), intern($name)))
})*
}}
}
fillout! {
has_sse, "sse";
has_sse2, "sse2";
has_sse3, "sse3";
has_ssse3, "ssse3";
has_sse41, "sse4.1";
has_sse42, "sse4.2";
has_avx, "avx";
has_avx2, "avx2";
has_neon, "neon";
has_vfp, "vfp";
}
}

let target_machine = create_target_machine(sess);

fn features_contain(sess: &Session, s: &str) -> bool {
sess.target.target.options.features.contains(s) || sess.opts.cg.target_feature.contains(s)
}
let whitelist = match &*sess.target.target.arch {
"arm" => ARM_WHITELIST,
"x86" | "x86_64" => X86_WHITELIST,
_ => &[],
};

pub fn has_sse(sess: &Session) -> bool {
features_contain(sess, "+sse") || has_sse2(sess)
}
pub fn has_sse2(sess: &Session) -> bool {
// x86-64 requires at least SSE2 support
sess.target.target.arch == "x86_64" || features_contain(sess, "+sse2") || has_sse3(sess)
}
pub fn has_sse3(sess: &Session) -> bool {
features_contain(sess, "+sse3") || has_ssse3(sess)
}
pub fn has_ssse3(sess: &Session) -> bool {
features_contain(sess, "+ssse3") || has_sse41(sess)
}
pub fn has_sse41(sess: &Session) -> bool {
features_contain(sess, "+sse4.1") || has_sse42(sess)
}
pub fn has_sse42(sess: &Session) -> bool {
features_contain(sess, "+sse4.2") || has_avx(sess)
}
pub fn has_avx(sess: &Session) -> bool {
features_contain(sess, "+avx") || has_avx2(sess)
}
pub fn has_avx2(sess: &Session) -> bool {
features_contain(sess, "+avx2")
}

pub fn has_neon(sess: &Session) -> bool {
// AArch64 requires NEON support
sess.target.target.arch == "aarch64" || features_contain(sess, "+neon")
}
pub fn has_vfp(sess: &Session) -> bool {
// AArch64 requires VFP support
sess.target.target.arch == "aarch64" || features_contain(sess, "+vfp")
let tf = InternedString::new("target_feature");
for feat in whitelist {
assert_eq!(feat.chars().last(), Some('\0'));
if unsafe { LLVMRustHasFeature(target_machine, feat.as_ptr() as *const c_char) } {
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to have an assertion that the strings end in \0 here, to ensure people don't make a mistake when adding features in future.

cfg.push(attr::mk_name_value_item_str(tf.clone(), intern(&feat[..feat.len()-1])))
}
}
}
7 changes: 7 additions & 0 deletions src/librustc_llvm/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ fn main() {
}
cfg.flag(flag);
}

for component in &components[..] {
let mut flag = String::from("-DLLVM_COMPONENT_");
flag.push_str(&component.to_uppercase());
cfg.flag(&flag);
}

cfg.file("../rustllvm/ExecutionEngineWrapper.cpp")
.file("../rustllvm/PassWrapper.cpp")
.file("../rustllvm/RustWrapper.cpp")
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2013,6 +2013,9 @@ extern {
pub fn LLVMRustFindAndCreatePass(Pass: *const c_char) -> PassRef;
pub fn LLVMRustAddPass(PM: PassManagerRef, Pass: PassRef);

pub fn LLVMRustHasFeature(T: TargetMachineRef,
s: *const c_char) -> bool;

pub fn LLVMRustCreateTargetMachine(Triple: *const c_char,
CPU: *const c_char,
Features: *const c_char,
Expand Down
32 changes: 1 addition & 31 deletions src/librustc_trans/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use std::str;
use std::sync::{Arc, Mutex};
use std::sync::mpsc::channel;
use std::thread;
use libc::{c_uint, c_int, c_void};
use libc::{c_uint, c_void};

pub fn llvm_err(handler: &errors::Handler, msg: String) -> ! {
match llvm::last_error() {
Expand Down Expand Up @@ -984,36 +984,6 @@ pub fn run_assembler(sess: &Session, outputs: &OutputFilenames) {
}
}

pub unsafe fn configure_llvm(sess: &Session) {
let mut llvm_c_strs = Vec::new();
let mut llvm_args = Vec::new();

{
let mut add = |arg: &str| {
let s = CString::new(arg).unwrap();
llvm_args.push(s.as_ptr());
llvm_c_strs.push(s);
};
add("rustc"); // fake program name
if sess.time_llvm_passes() { add("-time-passes"); }
if sess.print_llvm_passes() { add("-debug-pass=Structure"); }

// FIXME #21627 disable faulty FastISel on AArch64 (even for -O0)
if sess.target.target.arch == "aarch64" { add("-fast-isel=0"); }

for arg in &sess.opts.cg.llvm_args {
add(&(*arg));
}
}

llvm::LLVMInitializePasses();

llvm::initialize_available_targets();

llvm::LLVMRustSetLLVMOptions(llvm_args.len() as c_int,
llvm_args.as_ptr());
}

pub unsafe fn with_llvm_pmb(llmod: ModuleRef,
config: &ModuleConfig,
f: &mut FnMut(llvm::PassManagerBuilderRef)) {
Expand Down
20 changes: 0 additions & 20 deletions src/librustc_trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2709,26 +2709,6 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>,
tcx.sess.opts.debug_assertions
};

// Before we touch LLVM, make sure that multithreading is enabled.
unsafe {
use std::sync::Once;
static INIT: Once = Once::new();
static mut POISONED: bool = false;
INIT.call_once(|| {
if llvm::LLVMStartMultithreaded() != 1 {
// use an extra bool to make sure that all future usage of LLVM
// cannot proceed despite the Once not running more than once.
POISONED = true;
}

::back::write::configure_llvm(&tcx.sess);
});

if POISONED {
bug!("couldn't enable multi-threaded LLVM");
}
}

let link_meta = link::build_link_meta(&tcx, name);

let codegen_units = tcx.sess.opts.cg.codegen_units;
Expand Down
Loading