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

Rollup of 5 pull requests #55230

Merged
merged 8 commits into from
Oct 21, 2018
Merged

Rollup of 5 pull requests #55230

merged 8 commits into from
Oct 21, 2018

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

alexcrichton and others added 4 commits October 19, 2018 02:35
The issue of passing around SIMD types as values between functions has
seen [quite a lot] of [discussion], and although we thought [we fixed
it][quite a lot] it [wasn't]! This PR is a change to rustc to, again,
try to fix this issue.

The fundamental problem here remains the same, if a SIMD vector argument
is passed by-value in LLVM's function type, then if the caller and
callee disagree on target features a miscompile happens. We solve this
by never passing SIMD vectors by-value, but LLVM will still thwart us
with its argument promotion pass to promote by-ref SIMD arguments to
by-val SIMD arguments.

This commit is an attempt to thwart LLVM thwarting us. We, just before
codegen, will take yet another look at the LLVM module and demote any
by-value SIMD arguments we see. This is a very manual attempt by us to
ensure the codegen for a module keeps working, and it unfortunately is
likely producing suboptimal code, even in release mode. The saving grace
for this, in theory, is that if SIMD types are passed by-value across
a boundary in release mode it's pretty unlikely to be performance
sensitive (as it's already doing a load/store, and otherwise
perf-sensitive bits should be inlined).

The implementation here is basically a big wad of C++. It was largely
copied from LLVM's own argument promotion pass, only doing the reverse.
In local testing this...

Closes rust-lang#50154
Closes rust-lang#52636
Closes rust-lang#54583
Closes rust-lang#55059

[quite a lot]: rust-lang#47743
[discussion]: rust-lang#44367
[wasn't]: rust-lang#50154
when you try to edit a crate inside the compiler tree using rls, it
generates it's assets under target/rls, then tidy is trying to validate
line lenghts for C headers, etc
@Manishearth
Copy link
Member Author

@bors p=20 r+

@bors
Copy link
Contributor

bors commented Oct 20, 2018

📌 Commit b11933f08625140b4d8cd61f55b834bfd8f81b80 has been approved by Manishearth

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 20, 2018
@bors
Copy link
Contributor

bors commented Oct 20, 2018

⌛ Testing commit b11933f08625140b4d8cd61f55b834bfd8f81b80 with merge b313797f502a1b4e967cc8bcd5998253b2a3006b...

@bors
Copy link
Contributor

bors commented Oct 20, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 20, 2018
The issue of passing around SIMD types as values between functions has
seen [quite a lot] of [discussion], and although we thought [we fixed
it][quite a lot] it [wasn't]! This PR is a change to rustc to, again,
try to fix this issue.

The fundamental problem here remains the same, if a SIMD vector argument
is passed by-value in LLVM's function type, then if the caller and
callee disagree on target features a miscompile happens. We solve this
by never passing SIMD vectors by-value, but LLVM will still thwart us
with its argument promotion pass to promote by-ref SIMD arguments to
by-val SIMD arguments.

This commit is an attempt to thwart LLVM thwarting us. We, just before
codegen, will take yet another look at the LLVM module and demote any
by-value SIMD arguments we see. This is a very manual attempt by us to
ensure the codegen for a module keeps working, and it unfortunately is
likely producing suboptimal code, even in release mode. The saving grace
for this, in theory, is that if SIMD types are passed by-value across
a boundary in release mode it's pretty unlikely to be performance
sensitive (as it's already doing a load/store, and otherwise
perf-sensitive bits should be inlined).

The implementation here is basically a big wad of C++. It was largely
copied from LLVM's own argument promotion pass, only doing the reverse.
In local testing this...

Closes rust-lang#50154
Closes rust-lang#52636
Closes rust-lang#54583
Closes rust-lang#55059

[quite a lot]: rust-lang#47743
[discussion]: rust-lang#44367
[wasn't]: rust-lang#50154
…isdreavus

Sending this PR in today so we can see what linkchecker wants overnight; please don't r+ until travis is green!
…akis

Sometimes I just return `ty::List::empty()` because I cannot express these \"built-in\" clauses without bound tys support.

r? @nikomatsakis
when you try to edit a crate inside the compiler tree using rls, it
generates it's assets under target/rls, then tidy is trying to validate
line lenghts for C headers, etc
@Manishearth
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2018

📌 Commit f2848a0 has been approved by Manishearth

@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 Oct 20, 2018
@bors
Copy link
Contributor

bors commented Oct 20, 2018

⌛ Testing commit f2848a0 with merge d541876...

bors added a commit that referenced this pull request Oct 20, 2018
Rollup of 5 pull requests

Successful merges:

 - #55156 (Fixed: Multiple errors on single typo in match pattern)
 - #55189 (update books for the next release)
 - #55193 (make asm diagnostic instruction optional)
 - #55203 (Write an initial version of the `program_clauses` callback)
 - #55213 (ignore target folders)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Oct 21, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: Manishearth
Pushing d541876 to master...

@bors bors merged commit f2848a0 into rust-lang:master Oct 21, 2018
@bors bors mentioned this pull request Oct 21, 2018
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
@Manishearth Manishearth deleted the rollup branch June 22, 2020 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup 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