Skip to content
This repository was archived by the owner on Nov 21, 2018. It is now read-only.

Generate plain powerpc64 binaries #145

Closed
wants to merge 1 commit into from

Conversation

zeldin
Copy link

@zeldin zeldin commented Dec 21, 2016

The cross toolchain provided by Docker defaults to generate code
for Power ISA v.2.07 (POWER8). Select a generic profile instead.

The cross toolchain provided by Docker defaults to generate code
for Power ISA v.2.07 (POWER8).  Select a generic profile instead.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (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.

@alexcrichton
Copy link
Contributor

Thanks for the PR! Perhaps though this could be added to the gcc crate which is then automatically used and plumbed for both rustbuild and the rest of the ecosystem?

@zeldin
Copy link
Author

zeldin commented Dec 21, 2016

Well, it's your call. However:

  • The flag is only necessary when using the patched toolchain that Docker uses, not when using a normal powerpc64 toolchain
  • People building their own crates from C code might prefer a different CPU target if they have a particular deployment system in mind

@alexcrichton
Copy link
Contributor

We already pass similar flags for other architectures, and it seems totally reasonable that gcc-rs would have an option to override that. In that sense yeah let's go with patching gcc-rs rather than buildbot here.

@zeldin
Copy link
Author

zeldin commented Dec 27, 2016

Well, the

 // Turn codegen down on i586 to avoid some instructions.

bit seems like a very similar case, so I guess it could serve as a precedent.

@alexcrichton
Copy link
Contributor

Ah yeah that's a case for a whole new target, but we should regardless encode the defaults in gcc-rs anyway so we can work with a maximal number of powerpc compilers.

@cuviper
Copy link
Contributor

cuviper commented Jan 31, 2017

With #149, the cross-toolchain for powerpc64 should now default to -mcpu=power4 -mtune=power6, similar to RHEL6's toolchain. Is that sufficient for your needs?

@zeldin
Copy link
Author

zeldin commented Feb 1, 2017

@cuviper The latest nightly still gives me illegal instructions. Has this been deployed already?

@cuviper
Copy link
Contributor

cuviper commented Feb 1, 2017

Assuming your latest nightly is the same as mine, 2017-01-25, then no, that's before my PR was merged. Plus I think @brson mentioned he only deployed the actual new image yesterday.

See also rust-lang/rust#39382 with the same changes -- this is what builds will eventually use.

@zeldin
Copy link
Author

zeldin commented Feb 1, 2017

Ok, I'll wait for a new build to drop and test again then. Using -mcpu=power4 should be fine, that's what I normally build with.

@zeldin
Copy link
Author

zeldin commented Feb 8, 2017

@cuviper Still getting illegal instructions (lxvw4x) in the current nightly.
Version is 1.17.0-nightly (c49d10207 2017-02-07)
Any idea whether you stuff has gone in yet or not?

@cuviper
Copy link
Contributor

cuviper commented Feb 8, 2017

I thought it had, but I also see version deps up to GLIBC_2.22 in that nightly, so I guess not yet.

@alexcrichton ?

@alexcrichton
Copy link
Contributor

Soon! These changes aren't quite in nightlies yet but we plan on doing so in the next week or so

@cuviper
Copy link
Contributor

cuviper commented Feb 13, 2017

Using the test server announced here, I can at least confirm the ABI part.

$ RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup update nightly
[...]
$ rustc -Vv
rustc 1.17.0-nightly (81bd2675e 2017-02-12)
binary: rustc
commit-hash: 81bd2675eaf96396e363d63aa068b0a462ec5a6d
commit-date: 2017-02-12
host: powerpc64-unknown-linux-gnu
release: 1.17.0-nightly
LLVM version: 3.9

The highest symbol requirements are GCC_4.2.0 and GLIBC_2.11, which are fine for RHEL6 as I intended.

I don't know a complete way to verify the cpu target, but taking @zeldin's particular example, I see lots of lxvw4x in the beta with objdump -d librustc_llvm-*.so and none in this test nightly.

@alexcrichton
Copy link
Contributor

Yay!

@alexcrichton
Copy link
Contributor

(thanks for verifying!)

@zeldin
Copy link
Author

zeldin commented Feb 13, 2017

I confirm that the "new nightly" (1.17.0-nightly (81bd2675e 2017-02-12) ) is running ok for me; I can compile and run hello world without problems. In contrast, the "old nightly" (1.17.0-nightly (956e2bcba 2017-02-12) has the lxvw4x instructions.

@zeldin zeldin closed this Feb 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants