Skip to content

Never MIR inline functions with a different instruction set #81753

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 6, 2021
Merged

Never MIR inline functions with a different instruction set #81753

merged 1 commit into from
Feb 6, 2021

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Feb 4, 2021 •
edited
Loading

No description provided.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 4, 2021
@tmiasko
Copy link
Contributor Author

tmiasko commented Feb 4, 2021

Do you think adding a mir-opt test would be worth it, given that running it
would require a target with multiple instruction sets, so it will be
inconvenient to update in the future?

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2021

hmm... yea, the difficulty of updating the test seems to me like a too high cost for the gain of a fairly straight forward test

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 4, 2021

📌 Commit 14ce0e861427cc1c73ea47c06ef61585a6e52720 has been approved by oli-obk

@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 Feb 4, 2021
@Aaron1011
Copy link
Contributor

Is this required for soundness? If so, I think we should definitely add a test.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 5, 2021

@bors r- yes it's required for soundness...

@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 Feb 5, 2021
@tmiasko tmiasko force-pushed the inline-instruction-set branch from 14ce0e8 to 41d51ad Compare February 5, 2021 12:34
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
Checking which error codes lack tests...
Found 436 error codes
Found 0 error codes with no tests
Done!
tidy error: /checkout/src/test/mir-opt/inline/inline-instruction-set.rs: too many trailing newlines (2)
some tidy checks failed

command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build"
expected success, got: exit code: 1

@tmiasko tmiasko force-pushed the inline-instruction-set branch from 41d51ad to eb5e2d0 Compare February 5, 2021 12:47
@tmiasko
Copy link
Contributor Author

tmiasko commented Feb 5, 2021

Added a no_core test. That shouldn't be too onerous to update, unless someone builds LLVM without required target.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 5, 2021

@bors r+

holy 🐮 I guess we should make more use of no_core tests

@bors
Copy link
Collaborator

bors commented Feb 5, 2021

📌 Commit eb5e2d0 has been approved by oli-obk

@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 Feb 5, 2021
@m-ou-se m-ou-se mentioned this pull request Feb 5, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2021
@bors
Auto merge of rust-lang#81810 - m-ou-se:rollup-q3nborp, r=m-ou-se
cfba499 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#80011 (Stabilize `peekable_next_if`)
 - rust-lang#81580 (Document how `MaybeUninit<Struct>` can be initialized.)
 - rust-lang#81610 (BTreeMap: make Ord bound explicit, compile-test its absence)
 - rust-lang#81664 (Avoid a hir access inside get_static)
 - rust-lang#81675 (Make rustdoc respect `--error-format short` in doctests)
 - rust-lang#81753 (Never MIR inline functions with a different instruction set)
 - rust-lang#81795 (Small refactor with Iterator::reduce)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 728c955 into rust-lang:master Feb 6, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 6, 2021
@tmiasko tmiasko deleted the inline-instruction-set branch February 6, 2021 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.51.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tmiasko @oli-obk @bors @Aaron1011 @rust-log-analyzer @rust-highfive @rustbot