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

Add s390x support #36369

Merged
merged 1 commit into from
Sep 11, 2016
Merged

Add s390x support #36369

merged 1 commit into from
Sep 11, 2016

Conversation

uweigand
Copy link
Contributor

@uweigand uweigand commented Sep 9, 2016

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:

- 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>
@rust-highfive
Copy link
Collaborator

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.

@uweigand
Copy link
Contributor Author

uweigand commented Sep 9, 2016

Note that this PR includes and supersedes #36239.

@alexcrichton
Copy link
Member

@bors: r+ 19b8408

Thanks so much @uweigand, especially for the thoroughness! Hopefully we can get libstd and cross-compiled rustc up an running soon as well for s390x

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)
Copy link
Member

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?).

Copy link
Member

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

Copy link
Contributor Author

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 ...)

Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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 ...).

@bors
Copy link
Contributor

bors commented Sep 11, 2016

⌛ Testing commit 19b8408 with merge babc5a7...

@bors
Copy link
Contributor

bors commented Sep 11, 2016

💔 Test failed - auto-mac-64-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

On Sat, Sep 10, 2016 at 5:56 PM, bors notifications@github.com wrote:

💔 Test failed - auto-mac-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-mac-64-opt-rustbuild/builds/2431


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36369 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95FqoKByEtd1R1OIfqORM2aPoYfixks5qo1GxgaJpZM4J5fG0
.

bors added a commit that referenced this pull request Sep 11, 2016
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>
@bors
Copy link
Contributor

bors commented Sep 11, 2016

⌛ Testing commit 19b8408 with merge 4812cf3...

@bors bors merged commit 19b8408 into rust-lang:master Sep 11, 2016
@uweigand uweigand deleted the s390x branch September 11, 2016 23:37
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 this pull request may close these issues.

6 participants