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.

Sign up for GitHub

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

compiler_builtins: base conditional compilation on the specification #36512

Closed
wants to merge 2 commits into from
Closed

compiler_builtins: base conditional compilation on the specification #36512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
480d743
compiler_builtins: base conditional compilation on the specification
Sep 15, 2016
10ff03d
fix make tidy
Sep 16, 2016
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
1 change: 1 addition & 0 deletions src/libcompiler_builtins/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ core = { path = "../libcore" }

[build-dependencies]
gcc = "0.3.27"
rustc-cfg = { git = "https://github.com/japaric/rustc-cfg", branch = "llvm-target" }
Copy link
Member

@alexcrichton alexcrichton Sep 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I had some comments on the other PR, but I'm not 100% sure that this is buying us enough to pull in a dependency. Many of the .contains are now just relevant_piece == "foo", which is good to work over parsed forms but hasn't cause ambiguity problems in the past?

Copy link
Member Author

Choose a reason for hiding this comment

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

The rationale is to make this crate more robust at handling custom targets whose triple may be foo or any other arbitrary name. My main motivation was to handle Cortex-M targets which are currently customs target and may be named whatever by downstream users. But if those Cortex-M targets land in tree then I'm OK with not landing this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was talking to @japaric about this the other day. This basically does what the extra env vars from rust-lang/rfcs#1721 would do, so it can be considered a stop-gap perhaps.

43 changes: 30 additions & 13 deletions src/libcompiler_builtins/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,14 @@
//! far far less than working with compiler-rt's build system over time.

extern crate gcc;
extern crate rustc_cfg;

use std::collections::BTreeMap;
use std::env;
use std::path::Path;

use rustc_cfg::Cfg;

struct Sources {
// SYMBOL -> PATH TO SOURCE
map: BTreeMap<&'static str, &'static str>,
Expand Down Expand Up @@ -73,9 +76,23 @@ impl Sources {

fn main() {
let target = env::var("TARGET").unwrap();
let Cfg {
ref llvm_target,
ref target_arch,
ref target_os,
ref target_env,
ref target_vendor,
..
} = Cfg::new(&target).unwrap();
// FIXME(stage0) use `unwrap` instead of `unwrap_or`
// NOTE in the latest stable/beta release, `rustc --print cfg` doesn't include `llvm_target` in
// its output. In those cases simply fallback to the target triple, which is usually similar to
// llvm-target, as a workaround.
let llvm_target = llvm_target.as_ref().unwrap_or(&target).split('-').collect::<Vec<_>>();
let target_vendor = target_vendor.as_ref().unwrap();
let cfg = &mut gcc::Config::new();

if target.contains("msvc") {
if target_env == "msvc" {
// Don't pull in extra libraries on MSVC
cfg.flag("/Zl");

Expand Down Expand Up @@ -183,7 +200,7 @@ fn main() {
"umoddi3.c",
"umodsi3.c"]);

if !target.contains("ios") {
if target_os != "ios" {
sources.extend(&["absvti2.c",
"addtf3.c",
"addvti3.c",
Expand Down Expand Up @@ -226,7 +243,7 @@ fn main() {
"umodti3.c"]);
}

if target.contains("apple") {
if target_vendor == "apple" {
sources.extend(&["atomic_flag_clear.c",
"atomic_flag_clear_explicit.c",
"atomic_flag_test_and_set.c",
Expand All @@ -235,20 +252,20 @@ fn main() {
"atomic_thread_fence.c"]);
}

if !target.contains("windows") {
if target_os != "windows" {
sources.extend(&["emutls.c"]);
}

if target.contains("msvc") {
if target.contains("x86_64") {
if target_env == "msvc" {
if llvm_target[0] == "x86_64" {
sources.extend(&["x86_64/floatdidf.c", "x86_64/floatdisf.c", "x86_64/floatdixf.c"]);
}
} else {
if !target.contains("freebsd") {
if target_os != "freebsd" {
sources.extend(&["gcc_personality_v0.c"]);
}

if target.contains("x86_64") {
if target_arch == "x86_64" {
sources.extend(&["x86_64/chkstk.S",
"x86_64/chkstk2.S",
"x86_64/floatdidf.c",
Expand All @@ -259,7 +276,7 @@ fn main() {
"x86_64/floatundixf.S"]);
}

if target.contains("i386") || target.contains("i586") || target.contains("i686") {
if llvm_target[0] == "i386" || llvm_target[0] == "i586" || llvm_target[0] == "i686" {
sources.extend(&["i386/ashldi3.S",
"i386/ashrdi3.S",
"i386/chkstk.S",
Expand All @@ -279,7 +296,7 @@ fn main() {
}
}

if target.contains("arm") && !target.contains("ios") {
if target_arch == "arm" && target_os != "ios" {
sources.extend(&["arm/aeabi_cdcmp.S",
"arm/aeabi_cdcmpeq_check_nan.c",
"arm/aeabi_cfcmp.S",
Expand Down Expand Up @@ -315,7 +332,7 @@ fn main() {
"arm/umodsi3.S"]);
}

if target.contains("armv7") {
if llvm_target[0] == "armv7" {
sources.extend(&["arm/sync_fetch_and_add_4.S",
"arm/sync_fetch_and_add_8.S",
"arm/sync_fetch_and_and_4.S",
Expand All @@ -338,7 +355,7 @@ fn main() {
"arm/sync_fetch_and_xor_8.S"]);
}

if target.contains("eabihf") {
if llvm_target.last().unwrap().ends_with("eabihf") {
sources.extend(&["arm/adddf3vfp.S",
"arm/addsf3vfp.S",
"arm/divdf3vfp.S",
Expand Down Expand Up @@ -377,7 +394,7 @@ fn main() {
"arm/unordsf2vfp.S"]);
}

if target.contains("aarch64") {
if target_arch == "aarch64" {
sources.extend(&["comparetf2.c",
"extenddftf2.c",
"extendsftf2.c",
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ pub fn default_configuration(sess: &Session) -> ast::CrateConfig {
let os = &sess.target.target.target_os;
let env = &sess.target.target.target_env;
let vendor = &sess.target.target.target_vendor;
let llvm_target = &sess.target.target.llvm_target;
let max_atomic_width = sess.target.target.options.max_atomic_width;

let fam = if let Some(ref fam) = sess.target.target.options.target_family {
Expand All @@ -949,6 +950,7 @@ pub fn default_configuration(sess: &Session) -> ast::CrateConfig {
mk(InternedString::new("target_pointer_width"), intern(wordsz)),
mk(InternedString::new("target_env"), intern(env)),
mk(InternedString::new("target_vendor"), intern(vendor)),
mk(InternedString::new("llvm_target"), intern(llvm_target)),
Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty weighty target-cfg to add that I'm not sure we'll want to do. Thoughts @brson?

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is not necessary. We can do conditional compilation for Cortex-M targets just fine without this change iff the Cortex-M are named thumbv* because we have to disambiguate betwen Cortex-M and our others ARM+Linux targets. This means we can no longer name them e.g. armv7m-none-eabi, which may be less strange to users, for who "thumb" may not ring a bell.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this direction actually. I think our target names should just be mere aliases (i.e. just the contents of the target definition matters). As @brson actually has mentioned elsewhere, or current policy of associating fine-grained attributes with triple (via the default targets) unnecessarily clashes with LLVM and other tool.s

];
match &fam[..] {
"windows" | "unix" => ret.push(attr::mk_word_item(fam)),
Expand Down
7 changes: 7 additions & 0 deletions src/rustc/std_shim/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.