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.

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

Add StatementKind::CopyNonOverlapping #77511

Merged
merged 10 commits into from
Mar 10, 2021
Merged

Add StatementKind::CopyNonOverlapping #77511

merged 10 commits into from
Mar 10, 2021

Conversation

JulianKnodt
Copy link
Contributor

@JulianKnodt JulianKnodt commented Oct 3, 2020 •
edited
Loading

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2020
@JulianKnodt JulianKnodt marked this pull request as draft October 3, 2020 21:34
nagisa
nagisa reviewed Oct 3, 2020
View reviewed changes
compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/mir/statement.rs Outdated Show resolved Hide resolved
@JulianKnodt
Copy link
Contributor Author

I've added placeholders to match statements on StatementKind, I'm not really sure how to properly implement most of them, but I put in some best guesses in some spots.

@bors
Copy link
Contributor

bors commented Oct 9, 2020

☔ The latest upstream changes (presumably #77747) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

nagisa
nagisa reviewed Oct 9, 2020
View reviewed changes
compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/mir/statement.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/mir/visit.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/mir/visit.rs Outdated Show resolved Hide resolved
compiler/rustc_mir/src/borrow_check/invalidation.rs Outdated Show resolved Hide resolved
@JulianKnodt JulianKnodt force-pushed the st_kind_cpy branch 3 times, most recently from cfd2bef to 4fd7b77 Compare October 10, 2020 00:08
@bors
Copy link
Contributor

bors commented Nov 6, 2020

☔ The latest upstream changes (presumably #78267) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@nagisa nagisa 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2020
@JulianKnodt
Copy link
Contributor Author

Sorry for the extremely long delay, for some reason I thought this was blocked on something, my bad

@JulianKnodt JulianKnodt force-pushed the st_kind_cpy branch 3 times, most recently from 31a543e to 318b63f Compare January 3, 2021 01:02
@nagisa
Copy link
Member

nagisa commented Jan 3, 2021

I thought this was blocked on something, my bad

At this point I'm just waiting for this to become non-WIP/non-draft before reviewing further. It seemed fairly complete last time I looked at the code changes proposed here.

@JulianKnodt JulianKnodt marked this pull request as ready for review January 4, 2021 01:15
@JulianKnodt JulianKnodt changed the title [WIP] Add StatementKind::CopyNonOverlapping Add StatementKind::CopyNonOverlapping Jan 4, 2021
@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 19, 2021
nagisa
nagisa reviewed Jan 22, 2021
View reviewed changes
compiler/rustc_codegen_ssa/src/mir/statement.rs Outdated Show resolved Hide resolved
compiler/rustc_mir/src/borrow_check/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_mir/src/borrow_check/invalidation.rs Outdated Show resolved Hide resolved
compiler/rustc_mir/src/borrow_check/type_check/mod.rs Outdated Show resolved Hide resolved
@JulianKnodt
Update match branches
89f45ed 
This updates all places where match branches check on StatementKind or UseContext.
This doesn't properly implement them, but adds TODOs where they are, and also adds some best
guesses to what they should be in some cases.
@JulianKnodt
Update interpret step
37a6c04
@JulianKnodt
Update cranelift
982382d
@JulianKnodt
Replace todos with impls
049045b 
Changed to various implementations, copying the style of prior function calls in places I was
unsure of.

Also one minor style nit.
@JulianKnodt
Change CopyNonOverlapping::codegen_ssa
845e4b5 
Fixes copy_non_overlapping codegen_ssa to properly handle pointees,
and use bytes instead of elem count
@JulianKnodt
Build StKind::CopyOverlapping
d4ae9ff 
This replaces where it was previously being constructed in intrinsics, with direct construction
of the Statement.
@JulianKnodt
Switch to changing cp_non_overlap in tform
217ff6b 
It was suggested to lower this in MIR instead of ssa, so do that instead.
@JulianKnodt
Copy link
Contributor Author

@rustbot label: +S-waiting-on-review -S-waiting-on-author

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 9, 2021
RalfJung
RalfJung approved these changes Mar 9, 2021
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

👍 for the interpreter changes modulo the nit I raised. Thanks a lot :)

@JulianKnodt
Clean up todos
4bceb29 
Also add some span_bugs where it is unreachable
@wesleywiser
Copy link
Member

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 9, 2021

📌 Commit 4bceb29 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 Mar 9, 2021
@JohnTitor JohnTitor mentioned this pull request Mar 9, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2021
@bors
Auto merge of rust-lang#82953 - JohnTitor:rollup-8rtk5g2, r=JohnTitor
861872b 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#77511 (Add StatementKind::CopyNonOverlapping)
 - rust-lang#79208 (Stabilize `unsafe_op_in_unsafe_fn` lint)
 - rust-lang#82411 (Fixes to ExitStatus and its docs)
 - rust-lang#82733 (Add powerpc-unknown-openbsd target)
 - rust-lang#82802 (Build rustdoc for run-make tests, not just run-make-fulldeps)
 - rust-lang#82849 (Add Option::get_or_default)
 - rust-lang#82908 (:arrow_up: rust-analyzer)
 - rust-lang#82937 (Update README.md to use the correct cmake version number)
 - rust-lang#82938 (Bump tracing-tree dependency)
 - rust-lang#82942 (Don't hardcode the `v1` prelude in diagnostics, to allow for new preludes.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 25fd504 into rust-lang:master Mar 10, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 10, 2021
@JulianKnodt JulianKnodt deleted the st_kind_cpy branch March 10, 2021 04:22
RalfJung
RalfJung reviewed Mar 10, 2021
View reviewed changes
compiler/rustc_mir/src/interpret/step.rs
self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?;

let dst =
self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?;
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed that this is wrong -- the size used for the access check here needs to be the full size, already multiplied with the count!

I'll submit a PR.

Copy link
Member

Choose a reason for hiding this comment

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

Done: #82967

@RalfJung RalfJung mentioned this pull request Mar 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2021
@bors
Auto merge of rust-lang#82967 - RalfJung:copy-nonoverlap, r=oli-obk
a4d9624 
fix copy_nonoverlapping

Fixes a bug introduced by rust-lang#77511

r? `@oli-obk`
Fixes rust-lang#82961
@RalfJung RalfJung mentioned this pull request Mar 10, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 10, 2021
@Dylan-DPC
Rollup merge of rust-lang#82976 - RalfJung:copy-nonoverlapping, r=oli…
f5196ae 
…-obk

fix error message for copy(_nonoverlapping) overflow

Fixes an error message regression introduced in rust-lang#77511 (and adds tests).

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung approved these changes

@oli-obk oli-obk oli-obk requested changes

@nagisa nagisa nagisa left review comments

@m-ou-se m-ou-se m-ou-se left review comments

@bjorn3 bjorn3 bjorn3 requested changes

@tmiasko tmiasko tmiasko left review comments

@Deloper2297 Deloper2297 Deloper2297 approved these changes

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.52.0
Development

Successfully merging this pull request may close these issues.