The 1.27 rust-stable bootstrap build step fails on mips64 #52108
Comments
stokhos
added
the
C-feature-request
|
cc @michaelwoerister @rust-lang/compiler -- marking a stable/stable regression. I'm not sure what could cause this (bootstrap is fairly small). |
Mark-Simulacrum
added
T-compiler
|
visiting for triage. Some notes, in part to self: mips64 is a tier 2 platform. We support building the tools (but not testing them) for such platforms ... and unfortunately we are doing such building via cross-compiles, so our continuous-integration system didn't catch whatever caused the build of bootstrap to start failing. Marking at P-medium for now. @draganmladjenovic I think we would be happy to look at any PR you provide that gives a way to sidestep the issue you are facing as long as it does not change the default behavior.... |
|
GCC MIPS documentation suggests that you should use |
|
@sanxiyn sh-4.4# ls Output |
|
@jiangtao9999 I haven't tested it but I think you should use RUSTFLAGS="-C llvm-args=-mxgot". |
|
@draganmladjenovic |
|
With this patch, rustc can build on mips64el. Index: rustc-1.27.1+dfsg1/src/bootstrap/bootstrap.py
===================================================================
--- rustc-1.27.1+dfsg1.orig/src/bootstrap/bootstrap.py
+++ rustc-1.27.1+dfsg1/src/bootstrap/bootstrap.py
@@ -591,6 +591,8 @@ class RustBuild(object):
(os.pathsep + env["LIBRARY_PATH"]) \
if "LIBRARY_PATH" in env else ""
env["RUSTFLAGS"] = "-Cdebuginfo=2"
+ if self.build_triple().startswith('mips'):
+ env["RUSTFLAGS"] += " -Cllvm-args=-mxgot"
env["PATH"] = os.path.join(self.bin_root(), "bin") + \
os.pathsep + env["PATH"]
if not os.path.isfile(self.cargo()): |
|
Is this needed all the time? (What's special about the bootstrap process?) If it's needed all the time, perhaps rustc should always be passing that flag to LLVM? In which case the change should be made to a more internal file, bootstrap.py only applies when compiling rustc itself. |
|
(By "all the time" I mean "including when compiling other rust programs not the rustc compiler to mips") |
|
I think what's special about rustc is that it is compiled with dynamic linking. Rust defaults to static linking, and for static linking I think always passing |
|
@sanxiyn @infinity0 But static linking will affected too because the use of GOT is tied to use of PIC code on Mips platform https://godbolt.org/g/cwU5Xc. I'd rather not do this by default or at least limit its use to debug builds. In the end it is a crutch that it's used when number of GOT entries per object exceeds 8K/16K on mips64 and mips32 respectively. Because the problem right now is limited to building of the bootstrap tool and nothing else (Actually I'm not sure for tests because I've run them with optimize-tests=true) I thought of adding of new env variable like BOOTSTRAP_RUSTFLAGS that will allow the users to control RUSTFLAGS for bootstrap tool build. |
|
Ah yes, it's PIC, not dynamic linking. On the other hand, do we build static linking with PIC? |
Don't know for sure but it looks that choice of relocation-model is orthogonal to the choice of linking. I believe the -linux- targets default to PIC which should cover all cases. There are some limitation in practice tho. For example the relocation-model=static code on mips64 cannot participate in dynamic linking. |
|
MIPS SVR4 ABI insist all userspace codes use PIC, even static linking, for GNU/MIPS, there is an extension for exec to use non-pic(since exec loaded at fixed position), which only works on N32/O32 ABI; |
it worked like a charm |
|
Hi everyone involved in the discussion,
This is on a Loongson 3A4000 box running How about we just go ahead and apply the EDIT: realized the flag setting is only for building |
|
Something about 1.46.0 (current beta) is breaking this patch: https://buildd.debian.org/status/fetch.php?pkg=rustc&arch=mips64el&ver=1.46.0%7Ebeta.2%2Bdfsg1-1%7Eexp4&stamp=1597142242&raw=0 |
|
--- rust.orig/src/bootstrap/bootstrap.py
+++ rust/src/bootstrap/bootstrap.py
@@ -681,6 +681,8 @@ class RustBuild(object):
# preserve existing RUSTFLAGS
env.setdefault("RUSTFLAGS", "")
env["RUSTFLAGS"] += " -Cdebuginfo=2"
+ if self.build_triple().startswith('mips'):
+ env["RUSTFLAGS"] += " -Ctarget-feature=+xgot"
build_section = "target.{}".format(self.build_triple())
target_features = [] |
|
@xen0n We are running into this problem now for the cargo build - attempt 1 attempt 2 attempt 3. The error messages are from the hashbrown crate, although I am having trouble reproducing it on a different mips64el machine. I am thinking of just adding "+xgot" to |
GOT loads with It might be possible to generate code like |
|
I agree with xen0n. We meet more and more packages in Debian needing xgot. |
|
I might need some time to setup a local benchmark environment, as I haven't been doing much rustc development lately. Who's going to own the work? |
|
Thanks for the explanation. Setting this feature unconditionally is just a 1-line patch which I am happy to apply in Debian, it is trivial work and I can own that. Or did you mean you wanted to do more things like testing? |
|
@infinity0 I'd prefer to benchmark rustc's performance before and after the global switch, if somehow we could successfully build rustc without the patch (might have to do the benchmark with a previous version of rustc). Or popular applications (think This benchmark work is not meant to block the one-liner PR, though. It's just a suggestion to help fellow developers on MIPS better understand the situation. |
|
OK that makes sense. Currently in Debian we are successfully building rustc without the global switch, and to my knowledge we are only applying it for the rustc builder program ("bootstrap"/"x.py") and now cargo 0.47. So, I could apply the global switch to the current version of rustc in Debian so you can compare the same version 1.48.0 with (1.48.0-dfsg1-2) or without the switch (1.48.0-dfsg1-1). They will remain available on snapshot.debian.org even after the next upgrade pushes them out of the main Debian archive. If you want to benchmark other programs, I guess it might be possible to disable the feature (after the global switch) with |
Sure, thanks for the information!
Yes that's the way to do it. I'm not quite sure if linking with |
|
Did anyone ever investigate this? We are still patching in +xgot on Debian which makes a recently-enabled test (#83485) fail: failuresAccording to @jrtc27 this difference is exactly what is expected when +xgot is enabled, so for now I will disable this test in Debian. |
|
Uhh... Does someone want to send in a PR? A few different solutions have been proposed by now and it's not actually clear which one is the necessary/desired one. |
…jackh726 Fix linkage for large binaries on mips64 platforms This pull request fixes the linkage for large binaries on mips64 platforms by enabling the `xgot` feature in LLVM. It is well understood that the generated binary will gain a hefty performance penalty where the external symbol jumps now cost at least three instructions each. Also, this pull request does not address the same issue on the mips32 counterparts (due to being unable to test the changes thoroughly). Should fix rust-lang#52108
…jackh726 Fix linkage for large binaries on mips64 platforms This pull request fixes the linkage for large binaries on mips64 platforms by enabling the `xgot` feature in LLVM. It is well understood that the generated binary will gain a hefty performance penalty where the external symbol jumps now cost at least three instructions each. Also, this pull request does not address the same issue on the mips32 counterparts (due to being unable to test the changes thoroughly). Should fix rust-lang#52108
…jackh726 Fix linkage for large binaries on mips64 platforms This pull request fixes the linkage for large binaries on mips64 platforms by enabling the `xgot` feature in LLVM. It is well understood that the generated binary will gain a hefty performance penalty where the external symbol jumps now cost at least three instructions each. Also, this pull request does not address the same issue on the mips32 counterparts (due to being unable to test the changes thoroughly). Should fix rust-lang#52108
The error mesage is:
Not sure yet if there is a bug behind this. Posting this here just in case.
What happens is that we hit the limit of 8K GOT entries per single object file. I've tried using 1.25 instead of 1.26 as bootstrap compiler and the bootstrap build passes with offending object file having ~7K symbols needing GOT entry. I haven't been able to find exact reason for this. At first glance, while it sound counterintuitive, it seems that we have more inlining on 1.26 which pulls in more symbol references.
I was able to sidestep this by increasing opt-level for bootstrap build to 3. As a stopgap, is it ok to provide and option to bootstrap.py to build bootstrap tool in release mode ? I can send the PR.
The text was updated successfully, but these errors were encountered: