-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Change x64 size checks to not apply to x32. #82841
Conversation
Rust contains various size checks conditional on target_arch = "x86_64", but these checks were never intended to apply to x86_64-unknown-linux-gnux32. Add target_pointer_width = "64" to the conditions.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (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. |
It seems a nice catch. |
For some background, I have had this ready for a while but waited with submitting this because Rust could not build successfully for x32 anyway because of LLVM bugs, so I felt there was little point. I had been working on the LLVM side of things to fix those bugs first and got them into LLVM 12, which as of #81451 is used for Rust, and have confirmed that now with this PR rustc is capable of successfully building itself for x32. |
This seems likely to be an extremely widespread issue in the ecosystem. I don't know if it's too late for this, but would it potentially make sense to make x32 have a distinct value for Between that and the various upstream indications that x32 has limited ongoing support, I'd hate to see extensive patches like this for crates in the ecosystem, if we could avoid those somehow. |
Changing As far as I am aware the question on the kernel side, "can we drop x32?", was answered with "no". |
@hvdijk wrote:
That's what I was asking about. I didn't know to what degree there was existing Rust code relying on x32 being
It's definitely not being dropped on the kernel side, and I wasn't trying to suggest otherwise. I've just seen some of the originators of it suggest that they don't think support for it should be expanded any further. |
@bors r+ rollup |
📌 Commit 95e096d has been approved by |
Thanks for the clarification, I have not seen this message by its originators, only the message on lkml, I thought you were referring to that. For better or worse it has taken on a life of its own. For completeness, even after this is merged, a complete a build of the extended tools (a build with |
Change x64 size checks to not apply to x32. Rust contains various size checks conditional on target_arch = "x86_64", but these checks were never intended to apply to x86_64-unknown-linux-gnux32. Add target_pointer_width = "64" to the conditions.
Rollup of 8 pull requests Successful merges: - rust-lang#81127 (Improve sift_down performance in BinaryHeap) - rust-lang#81879 (Added #[repr(transparent)] to core::cmp::Reverse) - rust-lang#82048 (or-patterns: disallow in `let` bindings) - rust-lang#82731 (Bump libc dependency of std to 0.2.88.) - rust-lang#82799 (Add regression test for rust-lang#75525) - rust-lang#82841 (Change x64 size checks to not apply to x32.) - rust-lang#82883 (Update Cargo) - rust-lang#82887 (Update CONTRIBUTING.md) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Rust contains various size checks conditional on target_arch = "x86_64", but these checks were never intended to apply to x86_64-unknown-linux-gnux32. Add target_pointer_width = "64" to the conditions.