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

rustc-builtins: use the target *specification* (rather than its triple) in build.rs #35474

Closed
japaric opened this issue Aug 7, 2016 · 5 comments

Comments

@japaric
Copy link
Member

japaric commented Aug 7, 2016

follow up of #35021

rustc-builtins' build.rs uses $TARGET to decide which intrinsics to compile. This won't work well with custom targets because these targets can have arbitrary triples (e.g. cortex-m0) that don't reflect the actual target specification (ARM architecture, no OS, etc). To make rustc-builtins more robust we should use the target specification (target_arch, target_os, etc) instead of the triple. This will improve rustc-builtins's support of custom targets and should simplify its build.rs logic: if spec.target_arch == "arm" instead of if target.starts_with("arm") || target.starts_with("thumb").

There's no standard mechanism to access the target specification from a build script but we could use an external crate like @nagisa's target_build_utils.

cc @alexcrichton @brson

@nagisa
Copy link
Member

nagisa commented Aug 7, 2016

I was planning to write an RFC on the topic of built-in and custom targets overall. I’ll try to get to it once I finish my other in-progress stuff.

@alexcrichton
Copy link
Member

Would certainly be nice to have! This logic should probably all live in gcc-rs anyway, and I wouldn't mind piling on more parsing/configuration to that crate, but if @nagisa's got an RFC along these lines sounds good to me to wait for that as well

@japaric
Copy link
Member Author

japaric commented Aug 9, 2016

TIL: rustc --print cfg is a thing. We could use that.

@brson
Copy link
Contributor

brson commented Aug 11, 2016

rustc --print cfg seems fine.

japaric pushed a commit to japaric/rust that referenced this issue Sep 15, 2016
of the target rather than on its triple (i.e. its name).

As explained in rust-lang#35474, the current conditional compilation logic, which
is based on the target triple (e.g. x86_64-unknown-linux-gnu), is not
robust in the presence of custom targets as those can have arbitrary
triples (e.g. cortex-m3).

To fix that, this commit changes the conditional compilation logic to
use the specification of the target (e.g. target_arch, target_os, etc.).
For that, compiler_builtins build script now depends on the rustc-cfg
crate, whose role is to shell out to `rustc --print cfg` and return its
output parsed as a `struct`.

With the goal of completely removing any direct dependency of the
conditional compilation logic on the target triple, this commit also
exposes the llvm-target field of the target specification via `rustc
--print cfg`.

closes rust-lang#35474
japaric pushed a commit to japaric/rust that referenced this issue Sep 17, 2016
of the target rather than on its triple (i.e. its name).

As explained in rust-lang#35474, the current conditional compilation logic, which
is based on the target triple (e.g. x86_64-unknown-linux-gnu), is not
robust in the presence of custom targets as those can have arbitrary
triples (e.g. cortex-m3).

To fix that, this commit changes the conditional compilation logic to
use the specification of the target (e.g. target_arch, target_os, etc.).
For that, compiler_builtins build script now depends on the rustc-cfg
crate, whose role is to shell out to `rustc --print cfg` and return its
output parsed as a `struct`.

With the goal of completely removing any direct dependency of the
conditional compilation logic on the target triple, this commit also
exposes the llvm-target field of the target specification via `rustc
--print cfg`.

closes rust-lang#35474
@Mark-Simulacrum
Copy link
Member

From #36512 (comment), I believe this has landed "upstream" in the compiler-builtins repo, so I'm going to close this issue since it's not really tracking anything. For those interested, rust-lang/compiler-builtins#66 is probably the best place to watch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants