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

Always run intrinsics lowering pass #80040

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Dec 14, 2020

Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.

The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 14, 2020
@nagisa
Copy link
Member

nagisa commented Dec 14, 2020

r? @nagisa

@bors r+

@bors
Copy link
Contributor

bors commented Dec 14, 2020

📌 Commit 1b74a82 has been approved by nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned varkor Dec 14, 2020
@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 14, 2020
Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.

The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…nagisa

Always run intrinsics lowering pass

Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.

The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
@tmiasko tmiasko force-pushed the always-lower-intrinsics branch from 1b74a82 to a9ff4bd Compare December 15, 2020 20:29
@tmiasko
Copy link
Contributor Author

tmiasko commented Dec 15, 2020

I updated the test output to account for changes from #79922 (but I am leaving the discriminant_value intrinsic in place for now).

@Dylan-DPC-zz
Copy link

Dylan-DPC-zz commented Dec 15, 2020

uhm you updated it just after the rollup :D (next time better to r- it first )
ah nevermind :P

failed in CI anyway
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 15, 2020
@tmiasko
Copy link
Contributor Author

tmiasko commented Dec 15, 2020

@Dylan-DPC the current version should account for rollup CI failure.

@Dylan-DPC-zz
Copy link

@tmiasko yeh my bad. will r+ it

@bors r+

@bors
Copy link
Contributor

bors commented Dec 15, 2020

📌 Commit a9ff4bd has been approved by Dylan-DPC

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…Dylan-DPC

Always run intrinsics lowering pass

Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.

The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…Dylan-DPC

Always run intrinsics lowering pass

Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.

The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
@gThorondorsen
Copy link

cc @RalfJung and @oli-obk (part of @rust-lang/miri which I can't ping) per this comment. They might have objections and/or may need to adjust Miri accordingly.

@RalfJung
Copy link
Member

This LGTM, and thanks for the heads-up! I'll update Miri once this lands. When more intrinsics are added to the lowering pass, please let me know.

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 17, 2020
…laumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#80006 (BTreeMap: more expressive local variables in merge)
 - rust-lang#80022 (BTreeSet: simplify implementation of pop_first/pop_last)
 - rust-lang#80035 (Optimization for bool's PartialOrd impl)
 - rust-lang#80040 (Always run intrinsics lowering pass)
 - rust-lang#80047 (Use more symbols in rustdoc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1f5d8de into rust-lang:master Dec 17, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 17, 2020
@tmiasko tmiasko deleted the always-lower-intrinsics branch December 17, 2020 15:06
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Dec 18, 2020
bors added a commit to rust-lang/miri that referenced this pull request Dec 22, 2020
remove intrinsic that is now implemented in the rustc side

Since rust-lang/rust#80040, we can rely on the pass introduced in rust-lang/rust#79049 to lower away `forget`.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants