Skip to content

compiler: factor Windows x86-32 ABI impl into its own file #137363

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Feb 21, 2025

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize wildly different ABIs using if-else in the same function.

r? @ghost

@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 Feb 21, 2025
@workingjubilee
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2025
…, r=<try>

compiler: untangle Windows x86-32 ABI impl

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize different ABIs using if-else.

r? `@ghost`

try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: i686-mingw-1
try-job: i686-mingw-2
try-job: i686-mingw-3
@bors
Copy link
Collaborator

bors commented Feb 21, 2025

⌛ Trying commit b2be9cf with merge aa94ee0...

@bors
Copy link
Collaborator

bors commented Feb 21, 2025

☀️ Try build successful - checks-actions
Build commit: aa94ee0 (aa94ee0761d6849463a7aa4384c3983e02da6a18)

@workingjubilee
Copy link
Member Author

r? compiler

@workingjubilee workingjubilee changed the title compiler: untangle Windows x86-32 ABI impl compiler: factor Windows x86-32 ABI impl into its own file Mar 5, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Comment on lines 25 to 26
// According to Clang, everyone but MSVC returns single-element
// float aggregates directly in a floating-point register.
Copy link
Member

@jieyouxu jieyouxu Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so this seemed slightly sus, but it was added in #44066 and looks right. Original comment was

For struct FloatOne { double x; }, it appears that MSVC passes the parameter in RCX and returns in RAX, but mingw-gcc passes the parameter and return value both in XMM0. Mingw-clang does the same as MSVC, so I think this is a GCC bug.

reported to gcc in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82028 marked as dupe of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85667.

https://gcc.godbolt.org/z/YjG44bx1b

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno if you want to add another link, up to you.

@jieyouxu
Copy link
Member

jieyouxu commented Mar 5, 2025

Feel free to r=me unless u want to adjust the comment.

@jieyouxu jieyouxu assigned jieyouxu and unassigned davidtwco Mar 5, 2025
While it shares more than zero code with the SysV x86-32 ABI impl,
there is no particular reason to organize wildly different ABIs
using if-else in the same function.
@workingjubilee workingjubilee force-pushed the untangle-x86-abi-impl branch from b2be9cf to 9d8ce72 Compare March 5, 2025 23:12
@workingjubilee
Copy link
Member Author

Updated the comments.

@workingjubilee
Copy link
Member Author

@bors r=jieyouxu

Not that it's merging any time soon

@bors
Copy link
Collaborator

bors commented Mar 6, 2025

📌 Commit 9d8ce72 has been approved by jieyouxu

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 6, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Mar 6, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 7, 2025
…pl, r=jieyouxu

compiler: factor Windows x86-32 ABI impl into its own file

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize wildly different ABIs using if-else in the same function.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
…kingjubilee

Rollup of 12 pull requests

Successful merges:

 - rust-lang#136667 (Revert vita's c_char back to i8)
 - rust-lang#136780 (std: move stdio to `sys`)
 - rust-lang#137107 (Override default `Write` methods for cursor-like types)
 - rust-lang#137363 (compiler: factor Windows x86-32 ABI impl into its own file)
 - rust-lang#137528 (Windows: Fix error in `fs::rename` on Windows 1607)
 - rust-lang#137537 (Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do)
 - rust-lang#137777 (Specialize `OsString::push` and `OsString as From` for UTF-8)
 - rust-lang#137832 (Fix crash in BufReader::peek())
 - rust-lang#137904 (Improve the generic MIR in the default `PartialOrd::le` and friends)
 - rust-lang#138115 (Suggest typo fix for static lifetime)
 - rust-lang#138125 (Simplify `printf` and shell format suggestions)
 - rust-lang#138129 (Stabilize const_char_classify, const_sockaddr_setters)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 7, 2025
…pl, r=jieyouxu

compiler: factor Windows x86-32 ABI impl into its own file

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize wildly different ABIs using if-else in the same function.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Mar 7, 2025
…pl, r=jieyouxu

compiler: factor Windows x86-32 ABI impl into its own file

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize wildly different ABIs using if-else in the same function.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136642 (Put the alloc unit tests in a separate alloctests package)
 - rust-lang#137337 (Add verbatim linker to AIXLinker)
 - rust-lang#137363 (compiler: factor Windows x86-32 ABI impl into its own file)
 - rust-lang#137685 (self-contained linker: conservatively default to `-znostart-stop-gc` on x64 linux)
 - rust-lang#138000 (atomic: clarify that failing conditional RMW operations are not 'writes')
 - rust-lang#138063 (Improve `-Zunpretty=hir` for parsed attrs)
 - rust-lang#138137 (setTargetTriple now accepts Triple rather than string)
 - rust-lang#138173 (Delay bug for negative auto trait rather than ICEing)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#137337 (Add verbatim linker to AIXLinker)
 - rust-lang#137363 (compiler: factor Windows x86-32 ABI impl into its own file)
 - rust-lang#137537 (Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do)
 - rust-lang#137606 (add a "future" edition)
 - rust-lang#137957 (Remove i586-pc-windows-msvc)
 - rust-lang#138000 (atomic: clarify that failing conditional RMW operations are not 'writes')
 - rust-lang#138013 (Add post-merge analysis CI workflow)
 - rust-lang#138033 (rustdoc: Add attribute-related tests for rustdoc JSON.)
 - rust-lang#138137 (setTargetTriple now accepts Triple rather than string)
 - rust-lang#138173 (Delay bug for negative auto trait rather than ICEing)
 - rust-lang#138184 (Allow anyone to relabel `CI-spurious-*`)
 - rust-lang#138187 (remove clones)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3b595aa into rust-lang:master Mar 8, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 8, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2025
Rollup merge of rust-lang#137363 - workingjubilee:untangle-x86-abi-impl, r=jieyouxu

compiler: factor Windows x86-32 ABI impl into its own file

While it shares more than zero code with the SysV x86-32 ABI impl, there is no particular reason to organize wildly different ABIs using if-else in the same function.
@workingjubilee workingjubilee deleted the untangle-x86-abi-impl branch March 9, 2025 21:54
@workingjubilee
Copy link
Member Author

yay. Thanks for the review!

# for free to join this conversation on GitHub. Already have an account? # 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