Skip to content

Conversation

erikdesjardins
Copy link
Contributor

In #77434 (comment), @eddyb said:

I wonder if it makes sense to limit this to returns [...]

Let's do a perf run and find out.

It seems the extern "C" ABI will pass arguments up to 2*usize in registers: https://godbolt.org/z/n8E6zc. (modified from #26494 (comment))

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 30, 2020
Comment on lines +66 to +75
pub union UnionU128x2{a:(u128, u128)}
// CHECK: define void @test_UnionU128x2(i128 %_1.0, i128 %_1.1)
#[no_mangle]
pub fn test_UnionU128x2(_: UnionU128x2) { loop {} }

#[repr(C)]
pub union CUnionU128{a:u128}
// CHECK: define void @test_CUnionU128(%CUnionU128* {{.*}} %_1)
pub union CUnionU128x2{a:(u128, u128)}
// CHECK: define void @test_CUnionU128x2(%CUnionU128x2* {{.*}} %_1)
#[no_mangle]
pub fn test_CUnionU128(_: CUnionU128) { loop {} }
pub fn test_CUnionU128x2(_: CUnionU128x2) { loop {} }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was testing that single-field #[repr(C)] unions don't forward that field's ABI, but after this change CUnionU128 would get passed by value, making it impossible to distinguish between i128 [forwarded from inner field] vs i128 [passing entire union by value]. So I had to make it bigger.

@jonas-schievink
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Nov 30, 2020

⌛ Trying commit 0183b41 with merge 6b84c5f481a691b1da945054fe9c6311fca322dd...

@bors
Copy link
Collaborator

bors commented Nov 30, 2020

☀️ Try build successful - checks-actions
Build commit: 6b84c5f481a691b1da945054fe9c6311fca322dd (6b84c5f481a691b1da945054fe9c6311fca322dd)

@rust-timer
Copy link
Collaborator

Queued 6b84c5f481a691b1da945054fe9c6311fca322dd with parent 349b3b3, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (6b84c5f481a691b1da945054fe9c6311fca322dd): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@nagisa
Copy link
Member

nagisa commented Dec 2, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2020

📌 Commit 53943d6 has been approved by nagisa

@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 Dec 2, 2020
@bors
Copy link
Collaborator

bors commented Dec 2, 2020

⌛ Testing commit 53943d6 with merge a094ff9...

@bors
Copy link
Collaborator

bors commented Dec 2, 2020

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing a094ff9 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants