-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add s390x support #36369
Add s390x support #36369
Conversation
This adds support for building the Rust compiler and standard library for s390x-linux, allowing a full cross-bootstrap sequence to complete. This includes: - Makefile/configure changes to allow native s390x builds - Full Rust compiler support for the s390x C ABI (only the non-vector ABI is supported at this point) - Port of the standard library to s390x - Update the liblibc submodule to a version including s390x support - Testsuite fixes to allow clean "make check" on s390x Caveats: - Resets base cpu to "z10" to bring support in sync with the default behaviour of other compilers on the platforms. (Usually, upstream supports all older processors; a distribution build may then chose to require a more recent base version.) (Also, using zEC12 causes failures in the valgrind tests since valgrind doesn't fully support this CPU yet.) - z13 vector ABI is not yet supported. To ensure compatible code generation, the -vector feature is passed to LLVM. Note that this means that even when compiling for z13, no vector instructions will be used. In the future, support for the vector ABI should be added (this will require common code support for different ABIs that need different data_layout strings on the same platform). - Two test cases are (temporarily) ignored on s390x to allow passing the test suite. The underlying issues still need to be fixed: * debuginfo/simd.rs fails because of incorrect debug information. This seems to be a LLVM bug (also seen with C code). * run-pass/union/union-basic.rs simply seems to be incorrect for all big-endian platforms. Signed-off-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Note that this PR includes and supersedes #36239. |
CFG_STATIC_LIB_NAME_s390x-unknown-linux-gnu=lib$(1).a | ||
CFG_LIB_GLOB_s390x-unknown-linux-gnu=lib$(1)-*.so | ||
CFG_LIB_DSYM_GLOB_s390x-unknown-linux-gnu=lib$(1)-*.dylib.dSYM | ||
CFG_CFLAGS_s390x-unknown-linux-gnu := -m64 $(CFLAGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may need to add -march=z10
to the list of CFLAGS because the gcc we'll be using in the buildbots (the Ubuntu 16.04 one) uses zEC12 as its default arch. Unless the z10 and newer are 100% compatible with the zEC12 instruction set. @uweigand, do you know if that's the case? (We use these CFLAGS to compile C dependencies of std like jemalloc)
@alexcrichton we'll be using rustbuild to build std for this target. Does rustbuild pick up these CFLAGS or do they have to be set somewhere (e.g. in config.toml? or in the bootstrap crate?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah we'll need to modify gcc-rs to add this flag by default, or it can be added to src/bootstrap/lib.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the last few generations of System Z processors are z10, z196, zEC12, and z13. In this order, each more recent processor is 100% upwards compatible with all earlier ones. Or to put it the other way, you can link together objects compiled for any of the processors, as long as you run the resulting binary on the most recent one. Therefore, I don't think it is particularly necessary to force the C components of std to be built for z10.
(The exception might be if you also run into problems with the valgrind tests since valgrind doesn't yet support all zEC12 instructions. That might indeed be a reason to use an older march level, until valgrind is fixed ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or to put it the other way, you can link together objects compiled for any of the processors, as long as you run the resulting binary on the most recent one. Therefore, I don't think it is particularly necessary to force the C components of std to be built for z10.
Right, in the current state e.g. jemalloc will be compiled for the zEC12 (because that's gcc default arch/cpu level) and Rust code will be compiled for the z10 (because of the target definition) resulting in binaries that only work on zEC12 or newer (at least for std programs, which use/link-to jemalloc by default). Is that OK? I though that the intention of setting the target base cpu to z10 was to support processors as old as the z10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention was that the Rust upstream source code doesn't force use of a more recent processor on all users. In the context of a particular distribution, where the distro has chosen to require a particular minimum architecture level, then of course that minimum level also applies to Rust binaries (you cannot avoid this anyway, even if you build the C components of std with -march=z10, since it'll still link against libc.so itself, which was built with -march=zEC12 on Ubuntu).
However, if you build Rust on another distro, like RHEL 7 or SLES 12, both of which use a default arch level of z196, you don't want to artificially increase the required architecture level just because of the default level provided as part of the upstream Rust compiler sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uweigand oh one point that may be worth clarify I think @japaric means is that these flags are also used for the binary builds that we ourselves ship. For example for some platforms we disable a bunch of SSE-related things and for others we ratchet down the CPU to be a little older than it is by default. That way the binaries we ship end up being compatible with more systems.
If distros, though, use this same file to build their own binaries then they'll indeed be forced to use older cpus (like z10) by default as it's not easy to change the makefiles. This should be relatively ok, though because:
- This is only used for jemalloc really, and distros tend to do custom things with that anyway
- Otherwise we don't really have much C code (especially not perf critical)
- With rustbuild you should be able to add more flags and override default ones, so it may be able to override this when building for distros.
Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, if you'd shipping binaries yourself, it makes sense to target an older CPU. If the flag to do so needs to be hard-coded in the sources because it's difficult to override, then I guess that's the way is has to be ... In the end, I agree that this shouldn't matter much performance-wise.
B.t.w. if you indeed build binaries to ship that are intended to support multiple distros, then the march level isn't the only problem. You'll also need to watch out for glibc symbol version issues; if you build on Ubuntu, it's possible that the resulting binary won't run on RHEL or SLES since those have much older glibc versions. The general recommendation would be to build binaries on the oldest distro you want to support (in which case also the march problem may be solved as well ...).
⌛ Testing commit 19b8408 with merge babc5a7... |
💔 Test failed - auto-mac-64-opt-rustbuild |
@bors: retry On Sat, Sep 10, 2016 at 5:56 PM, bors notifications@github.com wrote:
|
Add s390x support This adds support for building the Rust compiler and standard library for s390x-linux, allowing a full cross-bootstrap sequence to complete. This includes: - Makefile/configure changes to allow native s390x builds - Full Rust compiler support for the s390x C ABI (only the non-vector ABI is supported at this point) - Port of the standard library to s390x - Update the liblibc submodule to a version including s390x support - Testsuite fixes to allow clean "make check" on s390x Caveats: - Resets base cpu to "z10" to bring support in sync with the default behaviour of other compilers on the platforms. (Usually, upstream supports all older processors; a distribution build may then chose to require a more recent base version.) (Also, using zEC12 causes failures in the valgrind tests since valgrind doesn't fully support this CPU yet.) - z13 vector ABI is not yet supported. To ensure compatible code generation, the -vector feature is passed to LLVM. Note that this means that even when compiling for z13, no vector instructions will be used. In the future, support for the vector ABI should be added (this will require common code support for different ABIs that need different data_layout strings on the same platform). - Two test cases are (temporarily) ignored on s390x to allow passing the test suite. The underlying issues still need to be fixed: * debuginfo/simd.rs fails because of incorrect debug information. This seems to be a LLVM bug (also seen with C code). * run-pass/union/union-basic.rs simply seems to be incorrect for all big-endian platforms. Signed-off-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
This adds support for building the Rust compiler and standard
library for s390x-linux, allowing a full cross-bootstrap sequence
to complete. This includes:
(only the non-vector ABI is supported at this point)
Caveats:
behaviour of other compilers on the platforms. (Usually, upstream
supports all older processors; a distribution build may then chose
to require a more recent base version.) (Also, using zEC12 causes
failures in the valgrind tests since valgrind doesn't fully support
this CPU yet.)
generation, the -vector feature is passed to LLVM. Note that this
means that even when compiling for z13, no vector instructions
will be used. In the future, support for the vector ABI should be
added (this will require common code support for different ABIs
that need different data_layout strings on the same platform).
the test suite. The underlying issues still need to be fixed:
This seems to be a LLVM bug (also seen with C code).
all big-endian platforms.
Signed-off-by: Ulrich Weigand ulrich.weigand@de.ibm.com