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

--target should ignore the machine part of the triplet in most cases #33147

Open
glandium opened this issue Apr 22, 2016 · 13 comments
Open

--target should ignore the machine part of the triplet in most cases #33147

glandium opened this issue Apr 22, 2016 · 13 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@glandium
Copy link
Contributor

Currently, rustc accepts a limited set of --target values. Those match some of the values one can get out of the typical config.guess, but for most platforms, the machine/manufacturer part of the target triplet should be ignored, or allowed to be omitted. (the output of config-guess is of the form CPU_TYPE-MANUFACTURER-OPERATING_SYSTEM or CPU_TYPE-MANUFACTURER-KERNEL-OPERATING_SYSTEM)

For example, the linux x86_64 target for rustc is --target=x86_64-unknown-linux-gnu.
Some time ago, config.guess would have returned that. Nowadays, it returns x86_64-pc-linux-gnu.
clang accepts both, as well as the shorter form: --target=x86_64-linux-gnu (in fact, it's very lax, --target=x86_64-foo works too).
A typical cross GCC toolchain will come as x86_64-linux-gnu-gcc too.

@alexcrichton
Copy link
Member

cc @brson, do you recall if there was any specific reason for doing exact target matching at some point in the past?

I believe LLVM has support for all of this under the hood, so it should be relatively easy to just follow what clang does here.

@nagisa
Copy link
Member

nagisa commented Apr 23, 2016

From the man pages:

The target triple has the general format <arch><sub>-<vendor>-<sys>-<abi>, where:

              <arch> x86, arm, thumb, mips, etc.

              <sub>  for example on ARM: v5, v6m, v7a, v7m, etc.

              <vendor>
                     pc, apple, nvidia, ibm, etc.

              <sys>  none, linux, win32, darwin, cuda, etc.

              <abi>  eabi, gnu, android, macho, elf, etc.

This matches the behaviour of LLVM.

To me it seems like <vendor> might have some purpose, as long as it is not unknown, but I do not agree we should allow to omit it even in the case of unknown.

Perhaps it would make sense to not allow target files to be for unknown vendor and, when a target with unknown vendor is requested, select any of the known vendors at random? (e.g. a pc for unknown in x86_64-unknown-linux-gnu)

That being said, first time hearing about config.guess. How significant is fraction of people who are (still) using that?

@ranma42
Copy link
Contributor

ranma42 commented Apr 27, 2016

config.guess is used by the configure script generated by autoconf, which is probably still very widely used. If you often build GNU projects from source, one of the ./configure steps you have run has very likely executed config.guess, but you never noticed because it is used internally and not explicitly run by the user (this script tries to guess the name of triple of the system which is running the build if --build is not specified).

We might also want to consider the behaviour of config.sub, i.e. the script which canonicalises the triple, as that might provide more insight about what parts of the triple should be ignored (as per the issue topic). I had originally suggested this in #31110.

I was originally planning to wait and fix the float code on x87, but as this topic is coming up independently, I started a thread in the internals forum as suggested by @nagisa.

@nagisa
Copy link
Member

nagisa commented Apr 27, 2016

We might also want to consider the behaviour of config.sub

I tried passing a bunch of varying options to config.sub and none of them omit the <vendor>. What I have seen being omitted, though, is <sys>… I think.

I also tried some intentionally pathological cases like x86_64-pear (where, say, pear is a fictional vendor producing fictional apple clones) only for the config.sub to produce a x86_64-pc-pear where you can't even tell whether pear is supposed to be a <sys> or <abi>.

Thus, my personal stance does not change and I feel rustc to be very reasonable by having users specify the exact quadruple.

suggested by @nagisa.

I don’t remember suggesting anything of the sort.

@ranma42
Copy link
Contributor

ranma42 commented Apr 27, 2016

@nagisa I asked about the best place to discuss the topic here and you replied in the following message.

@brson
Copy link
Contributor

brson commented Feb 3, 2017

I've heard this complaint recently. cc rust-lang/rfcs#1763.

@brson brson added I-needs-decision Issue: In need of a decision. T-tools T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 3, 2017
@brson
Copy link
Contributor

brson commented Feb 22, 2017

I think this is a bug and target specs should by default ignore the vendor string. I don't know offhand the impact on various Rust tooling, but it's not just rustc that would need to change.

@brson brson removed I-needs-decision Issue: In need of a decision. I-nominated labels Feb 22, 2017
@codyps
Copy link
Contributor

codyps commented Feb 22, 2017

I don't think trying to have rustc assign meaning to rust target strings is a good idea. Unconditionally ignoring the vendor will certainly break people who are using that part of the string to differentiate similar, but slightly different configurations (for example, in meta-rust the vendor is the distro name, and generally serves to indicate that it has a different set of linker flags to use a sysroot).

To ensure that our targets are fully configurable, we'd ideally have all the target.contains() gunk that is currently strewn around rustbuild changed to use configuration/target spec options. We know we can't represent all configurations in a simple string.

Right now, we've got a simple 1-to-1 mapping for target strings to configuration. Trying to parse the target string to treat one part of it differently gets rid of that and increases complexity without much benefit.

Now, that said: it could make sense to allow the build process for rust understand that if someone says "give me x86_64-pc-linux-gnu" without anything additional specified, that the person is asking for something very much like the builtin x86_64-unknown-linux-gnu rust target. As long as the mapping is only done by the already fairly auto-magic build process for rustc, it's unlikely to cause issues. But at the same time, I don't see such a mapping in rustbuild only being very useful.

@steveklabnik steveklabnik added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed A-compiler labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 24, 2017
@metux
Copy link

metux commented Jul 12, 2017

Note that vendor is used for differentiating between OS platforms/libc-types. (eg, "gnu", "uclibc", ...)
Don't try to be clever about that (no hidden magic) - just ask the target cc for the right tuple and take it as it is.
(the bootstrapping - as long as fetching binaries is still required - needs a mapping, of course).

@cuviper
Copy link
Member

cuviper commented Jul 12, 2017

The "gnu" part is the ABI; the vendor is "unknown".

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 25, 2017
@steveklabnik
Copy link
Member

Triage; no changes I'm aware of.

@str4d
Copy link
Contributor

str4d commented Jan 19, 2021

This is now an annoyance when using -Clinker-plugin-lto. We use a modern config.guess in zcashd's build system (where we link a Rust staticlib into C++ binaries), so clang is given a target of x86_64-pc-linux-gnu for the C++ code, while Rust continues to use x86_64-unknown-linux-gnu. At linker time, this causes multiple warnings of the form:

ld.lld: warning: Linking two modules of different target triples: ../target/x86_64-unknown-linux-gnu/release/librustzcash.a(rustzcash-41a277fd31e8d66d.rustzcash.4odm3xj7-cgu.0.rcgu.o at 42930)' is 'x86_64-unknown-linux-gnu' whereas 'libzcash.a(libzcash_a-Note.o at 4229688)' is 'x86_64-pc-linux-gnu'

I think this is technically a bug in lld (which should recognise these two targets as compatible), but the ability to tell rustc to use the modern target form would equivalently solve the problem.

@str4d
Copy link
Contributor

str4d commented Jan 19, 2021

I see that the target mismatch warnings were mentioned back in 2018 when -Clinker-plugin-lto was first being implemented (#49879 (comment)) but I can't find any subsequent mention of it anywhere.

str4d added a commit to str4d/zcash that referenced this issue Jan 20, 2021
Causes warnings at link time due to C++ and Rust using different target
strings. See rust-lang/rust#33147 for more
details.
zkbot added a commit to zcash/zcash that referenced this issue Jan 20, 2021
build: Enable cross-language ThinLTO

Causes warnings at link time due to C++ and Rust using different target
strings. These warnings do not break CI, because while we require `-Werror`
to build on Linux, that only affects compilation, not linking.

See rust-lang/rust#33147 for more details.
str4d added a commit to str4d/zcash that referenced this issue Apr 1, 2021
Causes warnings at link time due to C++ and Rust using different target
strings. See rust-lang/rust#33147 for more
details.
zkbot added a commit to zcash/zcash that referenced this issue Apr 3, 2021
build: Enable cross-language ThinLTO

Causes warnings at link time due to C++ and Rust using different target
strings. These warnings do not break CI, because while we require `-Werror`
to build on Linux, that only affects compilation, not linking.

See rust-lang/rust#33147 for more details.
str4d added a commit to str4d/zcash that referenced this issue Jun 16, 2023
Causes warnings at link time due to C++ and Rust using different target
strings. See rust-lang/rust#33147 for more
details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests