Skip to content

/rust

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

Fix C aggregate-passing ABI on powerpc #66050

Merged
merged 1 commit into from Nov 9, 2019

Conversation

@smaeul
Copy link
Contributor

smaeul commented Nov 3, 2019

The existing code (which looks like it was copied from MIPS) passes
aggregates by value in registers. This is wrong. According to the SVR4
powerpc psABI, all aggregates are passed indirectly.

See #64259 for more discussion, which addresses the ABI for the special
case of ZSTs (empty structs).
@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Nov 3, 2019

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)
@smaeul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

smaeul commented Nov 3, 2019 •
edited

r? @eddyb

This is the first commit from #64259 only.
@davidtwco

This comment has been minimized.

Sign in to view
Copy link
Member

davidtwco commented Nov 3, 2019

r? @eddyb
@rust-highfive rust-highfive assigned eddyb and unassigned davidtwco Nov 3, 2019
@eddyb
eddyb approved these changes Nov 3, 2019
View changes
@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Nov 3, 2019

@bors r+ Thanks!
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Nov 3, 2019

📌 Commit 06502d0 has been approved by eddyb
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Nov 5, 2019

☔️ The latest upstream changes (presumably #65938) made this pull request unmergeable. Please resolve the merge conflicts.
@smaeul smaeul force-pushed the smaeul:patch/powerpc-abi-2 branch from 06502d0 to 2c650e0 Nov 6, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Nov 6, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
2019-11-06T02:10:45.2470522Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-11-06T02:10:45.2634266Z ##[command]git config gc.auto 0
2019-11-06T02:10:45.2710713Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-11-06T02:10:45.2772120Z ##[command]git config --get-all http.proxy
2019-11-06T02:10:45.2909229Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/66050/merge:refs/remotes/pull/66050/merge
---
2019-11-06T02:20:35.0030407Z   local time: Wed Nov  6 02:20:35 UTC 2019
2019-11-06T02:20:35.0889911Z   network time: Wed, 06 Nov 2019 02:20:35 GMT
2019-11-06T02:20:35.0890696Z == end clock drift check ==
2019-11-06T02:20:36.5407499Z 
2019-11-06T02:20:36.5524171Z ##[error]Bash exited with code '1'.
2019-11-06T02:20:36.5550292Z ##[section]Starting: Checkout
2019-11-06T02:20:36.5552492Z ==============================================================================
2019-11-06T02:20:36.5552549Z Task         : Get sources
2019-11-06T02:20:36.5552616Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)
@smaeul
Fix C aggregate-passing ABI on powerpc
Loading status checks…
e648aa8 
The existing code (which looks like it was copied from MIPS) passes
aggregates by value in registers. This is wrong. According to the SVR4
powerpc psABI, all aggregates are passed indirectly.

See #64259 for more discussion, which addresses the ABI for the special
case of ZSTs (empty structs).
@smaeul smaeul force-pushed the smaeul:patch/powerpc-abi-2 branch from 2c650e0 to e648aa8 Nov 6, 2019
@smaeul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

smaeul commented Nov 8, 2019

@bors retry
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Nov 8, 2019

@smaeul: 🔑 Insufficient privileges: not in try users
@smaeul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

smaeul commented Nov 8, 2019

@eddyb I rebased, but I'm not sure how to get the PR moving again.
@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Nov 9, 2019

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Nov 9, 2019

📌 Commit e648aa8 has been approved by eddyb
Centril added a commit to Centril/rust that referenced this pull request Nov 9, 2019
@Centril
Rollup merge of rust-lang#66050 - smaeul:patch/powerpc-abi-2, r=eddyb
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
a1fc485 
Fix C aggregate-passing ABI on powerpc

The existing code (which looks like it was copied from MIPS) passes
aggregates by value in registers. This is wrong. According to the SVR4
powerpc psABI, all aggregates are passed indirectly.

See rust-lang#64259 for more discussion, which addresses the ABI for the special
case of ZSTs (empty structs).
@Centril Centril mentioned this pull request Nov 9, 2019
Merged
bors added a commit that referenced this pull request Nov 9, 2019
@bors
Auto merge of #66242 - Centril:rollup-h73ztr1, r=Centril
Loading status checks…
5a50275 
Rollup of 6 pull requests

Successful merges:

 - #65949 (Move promotion into its own pass)
 - #65994 (Point at where clauses where the associated item was restricted)
 - #66050 (Fix C aggregate-passing ABI on powerpc)
 - #66134 (Point at formatting descriptor string when it is invalid)
 - #66172 (Stabilize @file command line arguments)
 - #66226 (add link to unstable book for asm! macro)

Failed merges:

r? @ghost
@bors bors merged commit e648aa8 into rust-lang:master Nov 9, 2019
4 checks passed
4 checks passed
pr Build #20191106.4 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@smaeul smaeul deleted the smaeul:patch/powerpc-abi-2 branch Nov 11, 2019
@adamreichold adamreichold mentioned this pull request Nov 16, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb

Assignees

@eddyb eddyb

Labels
S-waiting-on-bors
Projects
None yet
Milestone
No milestone
5 participants
@smaeul @rust-highfive @davidtwco @eddyb @bors
You can’t perform that action at this time.