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

Flexible target specification #131

Merged
merged 4 commits into from
Jul 30, 2014
Merged

Flexible target specification #131

merged 4 commits into from
Jul 30, 2014

Conversation

emberian
Copy link
Member

@emberian emberian commented Jun 19, 2014

@mcpherrinm
Copy link

I assume all linked libraries in an executable should be compiled with the same -T settings. Would this be included in the crate metadata or something so the compiler can enforce it?

@emberian
Copy link
Member Author

I see no immediate reason why that should be enforced. I imagine any incompatibility would be caught by the linker. Do you have any example where you could cause very bad breakage with this, inside of a single --target, that you can't do today with -C?

@bharrisau
Copy link

Half of the -T flags are already in -C. Is there is benefit to adding the -T?

The two consumers of the target triple are LLVM and Rust, it looks like the target specification is for LLVM and the targ_os provides an override for Rust?

@emberian
Copy link
Member Author

-T sets defaults for the target. I really would prefer if we just loaded <target>.json from someplace.

@emberian
Copy link
Member Author

(In that, usually you don't use -T, but you might reasonably use -C)

@mcpherrinm
Copy link

I think it is likely to be a mistake to combine conflicting -T flags, and you may accidentally link against the wrong compiled artifacts, eg those with split stacks enabled. Messing around with the LLVM datalayout could also lead to badness.

@alexcrichton
Copy link
Member

The level of customizability here is definitely nice, and it seems like it could go pretty far. I'm worried about the power it gives you, however, because you all of a sudden can put yourself in some really weird situations.

Primarily, as @mcpherrinm noted, you can mix and match crates of all sorts of configuration, which can lead to perfectly safe code actually becoming unsafe in some circumstances (for example, stack overflow).

I share concerns with @bharrisau as well in that one of rustc's goals was to minimize the gargantuan command lines of C/C++. This seems to be expanding the command line for these project when they would be otherwise summarized in --target=foo.

It appears that "adding a new triple" is a real problem in the compiler due to the necessary support required. I wonder if the first stab at achieving the original goal would be to lower this bar of entry for new triples as much as possible. I imagine there are many things we could do to accept new triples.

I also think that focusing on target triples is itself quite powerful. We can easily have a target directory for all those crates, we can ensure that everything is compiled the same way, and you basically have a succinct alias for a form of configuration.

All in all, this is a very real problem, but I'm not sure if we want to resort to total customizability just yet?

@emberian
Copy link
Member Author

I really like the idea of the compiler just loading a <target>.json file to get a target config, but I don't like the idea of the compiler relying on external configuration out of the box, and not being able to pass flags to the compiler and having to put them in a file is a huge pet peeve of mine.

@emberian
Copy link
Member Author

I think anything less than a comprehensive target specification interface to the compiler (in some form) is going to be too limiting, and that we'll just end up with one anyway, but I'm not too enthralled with the precise interface here. In particular, -T feels bad. As to my concerns about requiring a file, maybe this is so rare that it doesn't matter.

@mcpherrinm
Copy link

Perhaps all target triples should be defined in a configuration file, say $PREFIX/local/rustc/triples.json that's distributed with the compiler, and new triples can be easily added to rustc by adding a new entry there, or to a local version, say in /etc/rustc/triples.json that overrides the default one.

@emberian
Copy link
Member Author

Well, I do want a way to specify a target config to use, for per-project stuff, but that I can see --target=path/to/target.json being repurposed for that. And thinking about it further, this is no more evil than a linker script, and probably just as common.

@bharrisau
Copy link

I do like the idea of moving some of the target specific stuff out of rustc. Data-layout is compiled in, and there is some magic that goes on to convert the specified target into the rust arch and rust os. Having the target spec in a distributed json (plus a method for a customised json) would make it easier to add new targets.

Does this RFC also want to include a --metal-triple configure argument or similar so libcore (plus others) can be built using the build tools?

@emberian
Copy link
Member Author

I don't think changes to configure are RFC-worthy.

@bharrisau
Copy link

@alexcrichton the biggest barrier to new triples is support for a default/undefined/unknown/metal OS in libstd (et al.).

@emberian
Copy link
Member Author

So for cfg'd off items in std etc I think what this describes (setting a targ_os) is fine. If std doesn't provide an implementation for your specific target, it's far easier to do that than change the compiler.

@bharrisau
Copy link

Looking at what I had to change to add an unknown OS in rust-lang/rust#12841:

  • Add data-layout to arm/mips/x86/x86_64
  • Add info to link.rs to work out DLL name
  • Update rpath.rs for dynamic linking on unknown OS
  • Update driver.rs so it turns *-unkown-* into unknown OS enum and the reverse for LLVM target
  • Update libstd::os to have naming consts for unknown OS
  • Update abi.rs to add Unknown to OS enum

This means that rustc supports a subset of the platforms that LLVM does.

I'm in favour of moving as much of this as possible out of rustc and just having relevant flags (dylib, segmented-stack, targ_arch name, targ_os name) set by a target descriptor. Some targets will have the descriptors bundled with rust (e.g. --target x86-linux-gnu uses the bundled x86-linux-gnu.json), and maybe a -C target-descriptor=xxx.json for custom targets?

Then I can set up a nvptx-cuda-unknown.json to support building for GPUs (or something like that).

Agree we need to be careful with hygiene between crates defined with different targets or descriptors. I've encountered a couple of ICE when mismatching crates.

@Ericson2314
Copy link
Contributor

I am getting the overall impression that this is adding another layer of abstraction to solve the exact problem target triples were supposed to solve. I am also nervous about the potential confusing of allowing triples not used by autotools / llvm / whoever came up with them in the first place.

Those more philosophical concerns aside, --target=<triple | path/to/config.json> does seem practical.

@emberian
Copy link
Member Author

Target triples are a good solution to a different problem. They allow you to precisely specify which environment a program is expected to run in. Unfortunately, they allow you to precisely specify which environment a program is expected to run in, not anything about that platform. You need to add a new triple to port something to a new platform. LLVM recognizes certain triples but basically only for the purposes of selecting which codegen backend to run, beyond that it doesn't care (not really, at least). The point original RFC made still stands, most projects will likely have to use x86_64-unknown-unknown or whichever, since anything else won't be accepted (or just fall back to unknown), but we need a way to specify certain things, both for rustc and to llvm, most importantly datalayout, segmented stack prologue support, and linker/linker arguments.

There is no canonical source of target triples. We can accept whatever we want as long as we hand LLVM something it can use. And I see no reason that we shouldn't.

@Ericson2314
Copy link
Contributor

In hindsight I feel a bit foolish. If all current + custom triples can be specified with the JSON file, then of course there is no overlap.

Also add `relocation-model`.
@emberian
Copy link
Member Author

Revised the RFC. A target configuration will only be loaded from a file. However, the builtin targets that rustc understands are still included in the compiler rather than in a separate file.

general mechanism for specifying certain characteristics of a target triple.
Redesign how targets are handled around this specification, including for the
built-in targets. Extend the `--target` flag to accept a file name of a target
specificatoin. A table of the target specification flags and their meaning:

Choose a reason for hiding this comment

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

:set spell

@zwarich
Copy link

zwarich commented Jun 27, 2014

@cmr LLVM will use the other components of target triples to make ABI decisions or decide to avoid certain features based on the supported feature set of the platform toolchain.

@errordeveloper
Copy link

This is pretty nifty! I tried adding arm-none-eabi and it was pretty tedious, and I'm yet unsure if it will work!

@bharrisau
Copy link

Is this just pending agreement on the config file format? I saw it discussed last week, but it isn't scheduled again this week.

@emberian
Copy link
Member Author

No, I've been ultra busy and haven't had time to either discuss this with
the others nor implement it.

On Thu, Jul 17, 2014 at 9:00 PM, Ben Harris notifications@github.com
wrote:

Is this just pending agreement on the config file format? I saw it
discussed last week, but it isn't scheduled again this week.


Reply to this email directly or view it on GitHub
#131 (comment).

http://octayn.net/

@errordeveloper
Copy link

Would it help if we propose something as a starting point?

@bharrisau
Copy link

Unlikely (wasn't my intention to trigger a bike-shedding cascade). Sounds like @cmr just hasn't had enough time to formalise the final details with the team: format (e.g. JSON - which we don't have an in-tree parser for), handling crate conflicts, and making sure everyone is comfortable specifying the LLVM target explicitly (and orthogonally to the rust target-arch and target-os).

@errordeveloper
Copy link

I understand that ideally JSON would be the best fit, however things can be simplified a little with CSV or even, may be, including a target.rs file that would either have a macro or just a struct defining the target, if providing JSON parser looks like a big leap...

@emberian
Copy link
Member Author

We already have JSON support as part of libserialize.

On Sun, Jul 20, 2014 at 8:18 AM, Ilya Dmitrichenko <notifications@github.com

wrote:

I understand that ideally JSON would be the best fit, however things can
be simplified a little with CSV or even, may be, including a target.rs
file that would either have a macro or just a struct defining the target,
if providing JSON parser looks like a big leap...


Reply to this email directly or view it on GitHub
#131 (comment).

http://octayn.net/

@errordeveloper
Copy link

@cmr I though @bharrisau meant that the JSON parser is not available for the compiler...

...JSON - which we don't have an in-tree parser for

@emberian
Copy link
Member Author

It is.

On Sun, Jul 20, 2014 at 8:36 AM, Ilya Dmitrichenko <notifications@github.com

wrote:

@cmr https://github.com/cmr I though @bharrisau
https://github.com/bharrisau meant that the JSON parser is not
available for the compiler...

...JSON - which we don't have an in-tree parser for


Reply to this email directly or view it on GitHub
#131 (comment).

http://octayn.net/

@bharrisau
Copy link

Yeah sorry, brain fart. Rustc outputs the ast as json, don't know what I
was thinking.

@brson
Copy link
Contributor

brson commented Jul 30, 2014

@mneumann
Copy link

As with RFC #185, a first step could be in centralizing target specific code to a single file. For porting Rust to DragonFly I had to make changes to the following files:

src/librustc/back/link.rs
src/librustc/driver/config.rs
src/librustc/metadata/loader.rs
src/librustc_back/arm.rs
src/librustc_back/mips.rs
src/librustc_back/mipsel.rs
src/librustc_back/x86.rs
src/librustc_back/rpath.rs
src/librustc_back/x86_64.rs
src/libsyntax/abi.rs

If we put all of this into a librustc_target crate it would simpliy porting a lot. We would then have a trait TargetSpec and for each supported target (e.g. TargetLinux, TargetFreeBSD) we'd provide an implementation for. And there would be a TargetGeneric which would pump the relevant values into rustc from command-line options or a JSON file. This all seems to be pretty straight-forward, but maybe I am missing something :).

@brson
Copy link
Contributor

brson commented Jul 30, 2014

Tracking bug is here rust-lang/rust#16093

@Centril Centril added A-target Target related proposals & ideas A-flags Proposals relating to rustc flags or flags for other tools. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-flags Proposals relating to rustc flags or flags for other tools. A-target Target related proposals & ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants