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

Support explicit 32-bit MIPS ABI for the synthetic object #113497

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

xSetech
Copy link
Contributor

@xSetech xSetech commented Jul 9, 2023

PR #95604 introduced a "synthetic object file to ensure all exported and used symbols participate in the linking". One constraint on this file is that for MIPS-based targets, its architecture-specific ELF flags must be the same as all other object files passed to the linker. That's enforced by LLD, here:
https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/lld/ELF/Arch/MipsArchTree.cpp#L77

The current approach to determining e_flags for 32-bit was implemented in PR #96930, which links to this issue that summarizes the problem well: ayrtonm/psx-sdk-rs#9

... the temporary object file is created with an e_flags which is
invalid for 32-bit MIPS targets. The main issue is that it omits the ABI
bits (EF_MIPS_ABI_O32) which implies it uses the N64 ABI.

To enable the N32 MIPS ABI (which succeeded O32), this patch enables setting the synthetic object's ABI based on the target "llvm-abiname" field, if it's given; otherwise, the O32 ABI is assumed for 32-bit MIPS targets.

More information about the N32 ABI can be found here: https://web.archive.org/web/20160121005457/http://techpubs.sgi.com/library/manuals/2000/007-2816-005/pdf/007-2816-005.pdf

@rustbot
Copy link
Collaborator

rustbot commented Jul 9, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 9, 2023
@rust-log-analyzer

This comment has been minimized.

PR rust-lang#95604 introduced a "synthetic object file to ensure all exported and
used symbols participate in the linking". One constraint on this file is
that for MIPS-based targets, its architecture-specific ELF flags must be
the same as all other object files passed to the linker. That's enforced
by LLD, here:
https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/lld/ELF/Arch/MipsArchTree.cpp#L77

The current approach to determining e_flags for 32-bit was implemented
in PR rust-lang#96930, which links to this issue that summarizes the problem well:
ayrtonm/psx-sdk-rs#9

> ... the temporary object file is created with an e_flags which is
> invalid for 32-bit MIPS targets. The main issue is that it omits the ABI
> bits (EF_MIPS_ABI_O32) which implies it uses the N64 ABI.

To enable the N32 MIPS ABI (which succeeded O32), this patch enables
setting the synthetic object's ABI based on the target "llvm-abiname"
field, if it's given; otherwise, the O32 ABI is assumed for 32-bit MIPS
targets.

More information about the N32 ABI can be found here:
https://web.archive.org/web/20160121005457/http://techpubs.sgi.com/library/manuals/2000/007-2816-005/pdf/007-2816-005.pdf
xSetech added a commit to xSetech/Raku that referenced this pull request Jul 10, 2023
Two MIPS III N32 ABI executables are produced; requires this patch:
rust-lang/rust#113497

See the readme for instructions and other notes.
xSetech added a commit to xSetech/Raku that referenced this pull request Jul 10, 2023
Two MIPS III N32 ABI executables are produced; requires this patch:
rust-lang/rust#113497

See the readme for instructions and other notes.
xSetech added a commit to xSetech/Raku that referenced this pull request Jul 10, 2023
Two MIPS III N32 ABI executables are produced; requires this patch:
rust-lang/rust#113497

See the readme for instructions and other notes.
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2023

📌 Commit 329e099 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#112717 (Implement a few more rvalue translation to smir)
 - rust-lang#113310 (Don't suggest `impl Trait` in path position)
 - rust-lang#113497 (Support explicit 32-bit MIPS ABI for the synthetic object)
 - rust-lang#113560 (Lint against misplaced where-clauses on associated types in traits)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 685ba08 into rust-lang:master Jul 11, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 11, 2023
xSetech added a commit to xSetech/Raku that referenced this pull request Jul 15, 2023
Contains a patch to use the N32 ABI:
rust-lang/rust#113497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants