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

inliner: Use substs_for_mir_body #78674

Merged
merged 1 commit into from
Nov 9, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Nov 2, 2020

Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the instance_mir returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use substs_for_mir_body to take that into account.

Resolves #78529.
Resolves #78560.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2020
@tmiasko tmiasko force-pushed the inline-substs-for-mir-body branch 3 times, most recently from 30892bf to 1fcde50 Compare November 2, 2020 22:54
@tmiasko tmiasko marked this pull request as ready for review November 2, 2020 22:55
@bors
Copy link
Contributor

bors commented Nov 3, 2020

☔ The latest upstream changes (presumably #78697) 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

@tmiasko tmiasko force-pushed the inline-substs-for-mir-body branch 2 times, most recently from aee7181 to 5fbd5a6 Compare November 3, 2020 12:01
@tmiasko tmiasko force-pushed the inline-substs-for-mir-body branch 3 times, most recently from 8e5aa92 to d2bc8a9 Compare November 3, 2020 20:47
@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Nov 4, 2020

📌 Commit d2bc8a9 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 Nov 4, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

@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 Nov 4, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

@bors delegate+

r=me with the documentation nit resolved.

@bors
Copy link
Contributor

bors commented Nov 4, 2020

✌️ @tmiasko can now approve this pull request

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Nov 4, 2020

📌 Commit d2bc8a9 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 Nov 4, 2020
Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use `substs_for_mir_body` to take that into account.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 6, 2020
… r=oli-obk

inliner: Use substs_for_mir_body

Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use `substs_for_mir_body` to take that into account.

Resolves rust-lang#78529.
Resolves rust-lang#78560.
@jyn514
Copy link
Member

jyn514 commented Nov 6, 2020

@bors r-

Failed in rollup:


---- [mir-opt] mir-opt/inline/inline-shims.rs stdout ----
9	      let mut _4: *mut std::vec::Vec<A>;   // in scope 0 at $DIR/inline-shims.rs:10:38: 10:39
10	      let mut _5: *mut std::option::Option<B>; // in scope 0 at $DIR/inline-shims.rs:11:38: 11:39
11	      scope 1 {
+	+         scope 3 (inlined drop_in_place::<Vec<A>> - shim(Some(Vec<A>))) { // at $DIR/inline-shims.rs:10:14: 10:40
+	+             let mut _6: &mut std::vec::Vec<A>; // in scope 3 at $DIR/inline-shims.rs:10:14: 10:40
+	+             let mut _7: ();              // in scope 3 at $DIR/inline-shims.rs:10:14: 10:40
+	+         }
12	      }
13	      scope 2 {
-	+         scope 3 (inlined drop_in_place::<Option<B>> - shim(Some(Option<B>))) { // at $DIR/inline-shims.rs:11:14: 11:40
-	+             let mut _6: isize;           // in scope 3 at $DIR/inline-shims.rs:11:14: 11:40
-	+             let mut _7: isize;           // in scope 3 at $DIR/inline-shims.rs:11:14: 11:40
+	+         scope 4 (inlined drop_in_place::<Option<B>> - shim(Some(Option<B>))) { // at $DIR/inline-shims.rs:11:14: 11:40
+	+             let mut _8: isize;           // in scope 4 at $DIR/inline-shims.rs:11:14: 11:40
+	+             let mut _9: isize;           // in scope 4 at $DIR/inline-shims.rs:11:14: 11:40
17	+         }
18	      }
19	  

21	          StorageLive(_3);                 // scope 0 at $DIR/inline-shims.rs:10:5: 10:42
22	          StorageLive(_4);                 // scope 1 at $DIR/inline-shims.rs:10:38: 10:39
23	          _4 = _1;                         // scope 1 at $DIR/inline-shims.rs:10:38: 10:39
-	          _3 = drop_in_place::<Vec<A>>(move _4) -> bb1; // scope 1 at $DIR/inline-shims.rs:10:14: 10:40
+	-         _3 = drop_in_place::<Vec<A>>(move _4) -> bb1; // scope 1 at $DIR/inline-shims.rs:10:14: 10:40
+	+         _6 = &mut (*_4);                 // scope 3 at $DIR/inline-shims.rs:10:14: 10:40
+	+         _7 = <Vec<A> as Drop>::drop(move _6) -> bb2; // scope 3 at $DIR/inline-shims.rs:10:14: 10:40
25	                                           // mir::Constant
-	                                           // + span: $DIR/inline-shims.rs:10:14: 10:37
-	                                           // + literal: Const { ty: unsafe fn(*mut std::vec::Vec<A>) {std::intrinsics::drop_in_place::<std::vec::Vec<A>>}, val: Value(Scalar(<ZST>)) }
+	-                                          // + span: $DIR/inline-shims.rs:10:14: 10:37
+	-                                          // + literal: Const { ty: unsafe fn(*mut std::vec::Vec<A>) {std::intrinsics::drop_in_place::<std::vec::Vec<A>>}, val: Value(Scalar(<ZST>)) }
+	+                                          // + span: $DIR/inline-shims.rs:10:14: 10:40
+	+                                          // + literal: Const { ty: for<'r> fn(&'r mut std::vec::Vec<A>) {<std::vec::Vec<A> as std::ops::Drop>::drop}, val: Value(Scalar(<ZST>)) }
28	      }
29	  
30	      bb1: {

36	-                                          // mir::Constant
37	-                                          // + span: $DIR/inline-shims.rs:11:14: 11:37
38	-                                          // + literal: Const { ty: unsafe fn(*mut std::option::Option<B>) {std::intrinsics::drop_in_place::<std::option::Option<B>>}, val: Value(Scalar(<ZST>)) }
-	+         _6 = discriminant((*_5));        // scope 3 at $DIR/inline-shims.rs:11:14: 11:40
-	+         switchInt(move _6) -> [0_isize: bb2, otherwise: bb3]; // scope 3 at $DIR/inline-shims.rs:11:14: 11:40
+	+         _8 = discriminant((*_5));        // scope 4 at $DIR/inline-shims.rs:11:14: 11:40
+	+         switchInt(move _8) -> [0_isize: bb3, otherwise: bb4]; // scope 4 at $DIR/inline-shims.rs:11:14: 11:40
41	      }
42	  
43	      bb2: {

+	+         drop(((*_4).0: alloc::raw_vec::RawVec<A>)) -> bb1; // scope 3 at $DIR/inline-shims.rs:10:14: 10:40
+	+     }
+	+ 
+	+     bb3: {
44	          StorageDead(_5);                 // scope 2 at $DIR/inline-shims.rs:11:39: 11:40
45	          return;                          // scope 0 at $DIR/inline-shims.rs:12:2: 12:2
46	+     }

47	+ 
-	+     bb3: {
-	+         drop((((*_5) as Some).0: B)) -> bb2; // scope 3 at $DIR/inline-shims.rs:11:14: 11:40
+	+     bb4: {
+	+         drop((((*_5) as Some).0: B)) -> bb3; // scope 4 at $DIR/inline-shims.rs:11:14: 11:40
50	      }
51	  }
52	  

thread '[mir-opt] mir-opt/inline/inline-shims.rs' panicked at 'Actual MIR output differs from expected MIR output /checkout/src/test/mir-opt/inline/inline_shims.drop.Inline.diff', src/tools/compiletest/src/runtest.rs:3272:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@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 Nov 6, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 6, 2020

@tmiasko you still have delegation rights, so r=me after a rebase

@tmiasko
Copy link
Contributor Author

tmiasko commented Nov 6, 2020

Ignored new tests on wasm target.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Nov 6, 2020

📌 Commit 8a8ee1a 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 Nov 6, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 8, 2020
… r=oli-obk

inliner: Use substs_for_mir_body

Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use `substs_for_mir_body` to take that into account.

Resolves rust-lang#78529.
Resolves rust-lang#78560.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#77640 (Refactor IntErrorKind to avoid "underflow" terminology)
 - rust-lang#78026 (Define `fs::hard_link` to not follow symlinks.)
 - rust-lang#78114 (Recognize `private_intra_doc_links` as a lint)
 - rust-lang#78228 (Promote aarch64-unknown-linux-gnu to Tier 1)
 - rust-lang#78345 (Fix handling of item names for HIR)
 - rust-lang#78437 (BTreeMap: stop mistaking node for an orderly place)
 - rust-lang#78476 (fix some incorrect aliasing in the BTree)
 - rust-lang#78674 (inliner: Use substs_for_mir_body)
 - rust-lang#78748 (Implement destructuring assignment for tuples)
 - rust-lang#78868 (Fix tab focus on restyled switches)
 - rust-lang#78878 (Avoid overlapping cfg attributes when both macOS and aarch64)
 - rust-lang#78882 (Nicer hunk headers for rust files)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b4589a8 into rust-lang:master Nov 9, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 9, 2020
@tmiasko tmiasko deleted the inline-substs-for-mir-body branch November 9, 2020 11:49
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Nov 27, 2020
… r=oli-obk

inliner: Use substs_for_mir_body

Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use `substs_for_mir_body` to take that into account.

Resolves rust-lang#78529.
Resolves rust-lang#78560.
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
7 participants