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

Upstream support for --print=target-cpus to LLVM #104785

Open
he32 opened this issue Nov 23, 2022 · 9 comments
Open

Upstream support for --print=target-cpus to LLVM #104785

he32 opened this issue Nov 23, 2022 · 9 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-specs Area: Compile-target specifications T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@he32
Copy link
Contributor

he32 commented Nov 23, 2022

The target-cpus feature is only exposed when building llvm through x.py because it depends on a patch in our fork of llvm. As a result, linking to an upstream llvm build with llvm.llvm-config = "..." won't work with --print=target-cpus. The relevant patch on the current branch is rust-lang/llvm-project@a8f170c; it would be nice if someone lobbied for that in upstream LLVM.

The last attempt to do that was https://reviews.llvm.org/D93789; the feedback there was to use fillValidCPUArchList() instead.

See below for the original issue description.


Hi,

is there a good reason release versions of rust do not support these options?

% rustc --print target-cpus
Target CPU help is not supported by this LLVM version.

% rustc --print target-spec-json
error: the `-Z unstable-options` flag must also be passed to enable the target-spec-json print option

OK, so let's try adding that option, then:

% rustc -Z unstable-options --print target-spec-json
error: the option `Z` is only accepted on the nightly compiler

Gah!
Googling a little gave a sliver of hope:

% rustc +nightly -Z unstable-options --print target-spec-json
error: the option `Z` is only accepted on the nightly compiler

% 

Nope!

% rustc --version
rustc 1.65.0
% uname
NetBSD
% uname -m
amd64
% uname -p
x86_64
% uname -r
9.99.104
% 

It does, though support rustc --print target-list, so this looks a little ... inconsistent?

The rustc(1) man page does mention both the above unsupported options without
any qualification.

I suspect this can relatively easily (and cheaply?) be improved?

@the8472
Copy link
Member

the8472 commented Nov 23, 2022

% rustc +nightly -Z unstable-options --print target-spec-json

The +<toolchain> syntax actually is rustup feature and not a rust feature. It requires that you installed rustup which provides the wrappers.

If the rustc and llvm build you're using are provided by the operating system's package manager then it likely is a stable version and won't support nightly features. In that case you should report the lack of target-cpus support to netbsd.

@he32
Copy link
Contributor Author

he32 commented Nov 23, 2022

% rustc +nightly -Z unstable-options --print target-spec-json

The +<toolchain> syntax actually is rustup feature and not a rust feature. It requires that you installed rustup which provides the wrappers.

Hm. I had the impression that rustup is the tool from the rust ecosystem which is used to thwart the effort of any package manager, so have avoided it like the plague. It is entirely possible that I misunderstood.

If the rustc and llvm build you're using are provided by the operating system's package manager then it likely is a stable version and won't support nightly features. In that case you should report the lack of target-cpus support to netbsd.

Yes, that would be nice to farm that question over to someone else to solve. However, the package manager for rust on NetBSD is presently ... me, and I do this alone in my "copious" spare time. We package releases. Therefore the question whether there is a good reason this functionality is turned off in the release versions. I still think it looks ... inconsistent with both itself (since target-list is available) and with the man page.

@the8472
Copy link
Member

the8472 commented Nov 23, 2022

The target-list feature is only exposed when building llvm through the rust build tools. Most likely because it depends on a patch in our fork of llvm. So if you're linking to an upstream llvm build it won't work.

@he32
Copy link
Contributor Author

he32 commented Nov 23, 2022

The target-list feature is only exposed when building llvm through the rust build tools. Most likely because it depends on a patch in our fork of llvm. So if you're linking to an upstream llvm build it won't work.

Ah, that invalidates my assumption that this would be "easy, cheap". Thanks for the explanation. I have a knob I can turn to turn on the build & use of the rust-internal LLVM, and it's presently turned off. I may return with a pull request to the man page...

@the8472
Copy link
Member

the8472 commented Nov 23, 2022

Hm. I had the impression that rustup is the tool from the rust ecosystem which is used to thwart the effort of any package manager, so have avoided it like the plague. It is entirely possible that I misunderstood.

Some linux distros ship a rustup package. You could add the system-rustc as toolchain to a preinstalled rustup so it can be selected as rustc +system ... and coexist with additional rustup-installed toolchains.

@cuviper
Copy link
Member

cuviper commented Nov 24, 2022

The relevant patch on the current branch is rust-lang/llvm-project@a8f170c

It would be nice if someone lobbied for that in upstream LLVM.
(Or something like it -- e.g. maybe it could return an iterator_range instead.)

@nikic
Copy link
Contributor

nikic commented Dec 7, 2022

@cuviper The last attempt to do that was https://reviews.llvm.org/D93789. The feedback there was to use fillValidCPUArchList() instead.

@workingjubilee
Copy link
Member

workingjubilee commented Feb 26, 2023

Hm. I had the impression that rustup is the tool from the rust ecosystem which is used to thwart the effort of any package manager, so have avoided it like the plague. It is entirely possible that I misunderstood.

Rustup is a program that serves as a Rust toolchain manager. It is used to retrieve various versions of the Rust toolchain. It has been extended with specific features to serve the interests of package managers before and more could be added.

EDIT: Oops, I repeated what people said previously.

Most notably, however, its existing features include being able to link arbitrary rustcs on the system with various aliases. Thus it not only makes sense for a distro to package rustup and expose it to the user actually running the OS, it may make sense to then go further to package a rustc, and have the rustup link it with a name like "distro", so that the user could install nightly and stable and whatnot, and build scripts could use the distro version of rustc and cargo via cargo +distro build, and so on.

@jyn514
Copy link
Member

jyn514 commented Apr 8, 2023

I'm going to repurpose this to track upstreaming target-cpus to LLVM. I'll open a separate issue for giving a better error when the first argument to rustc proper is a +toolchain.

@jyn514 jyn514 changed the title rustc releases are "unhelpful" (no target-cpus, no target-spec-json) Upstream support for --print=target-cpus to LLVM Apr 8, 2023
@jyn514 jyn514 added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-target-specs Area: Compile-target specifications labels Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-specs Area: Compile-target specifications T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants