Skip to content

Never MIR inline functions with a different instruction set#81753

Merged
bors merged 1 commit intorust-lang:masterfrom
tmiasko:inline-instruction-set
Feb 6, 2021
Merged

Never MIR inline functions with a different instruction set#81753
bors merged 1 commit intorust-lang:masterfrom
tmiasko:inline-instruction-set

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

Aaron1011 commented Feb 4, 2021

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

rust-log-analyzer commented Feb 5, 2021

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