Treat constant values as mir::ConstantKind::Val #94059

b-naber · 2022-02-16T17:31:48Z

Another step that is necessary for the introduction of Valtrees: we don't want to treat ty::Const instances of kind ty::ConstKind::Value as mir::ConstantKind::Ty anymore.

r? @oli-obk

oli-obk · 2022-02-16T20:55:55Z

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -570,6 +570,9 @@ impl<'tcx> Cx<'tcx> {

            hir::ExprKind::ConstBlock(ref anon_const) => {
                let anon_const_def_id = self.tcx.hir().local_def_id(anon_const.hir_id);
+
+                // FIXME Do we want to use `from_inline_const` once valtrees


I think that function is only used from here? . If so, we can move it onto constantkind and turn inline consts always into mir constants

Yes this is only used from here. But changing from_inline_const would require some changes in how we process patterns during thir construction. The next PR will be about changing thir to use mir::Constant instead of ty::Const, so would also include the change to from_inline_const. I would prefer to leave this as is in this PR, unless you want this change right now.

b-naber · 2022-02-18T15:50:33Z

How can I make the 'inline_into_box_place' test not fail in CI? A new alloc value (the bd09 in alloc[bd09]) is created in each CI run.

oli-obk · 2022-02-19T14:28:31Z

src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff

 -     }
 - 
 -     bb2: {
 +                                          // + span: $DIR/inline-into-box-place.rs:8:33: 8:43
 +                                          // + user_ty: UserType(0)
-+                                          // + literal: Const { ty: alloc::raw_vec::RawVec<u32>, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [65535], len: Size { raw: 16 } }, align: Align { pow2: 3 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) }
+                                          // + literal: Const { ty: alloc::raw_vec::RawVec<u32>, val: Unevaluated(Unevaluated { def: WithOptConstParam { did: DefId(5:77 ~ alloc[bd09]::raw_vec::{impl#0}::NEW), const_param_did: None }, substs: [u32], promoted: None }) }


Oof, yea that id is annoying. We should probably make the printing of unevaluted consts work in a more user-facing way. cc @lcnr

Afaict the issue here is that Debug for Unevaluated uses Debug for DefId which uses def_path_debug_str which prints the stable_crate_id. This ID is presumably not stable between targets so different CI runners disagree about what that ID should be.

It seems like - before this PR - all Unevaluated constants only referred to the currently compiled crate. The stabile_crate_id of which is probably stable for different targets.

Writing a potential fix rn.

This ID is presumably not stable between targets so different CI runners disagree about what that ID should be.

Indeed. Liballoc is compiled by cargo which afaik hashes the target into the value passed in using -Cmetadata. The -Cmetadata argumemts are one of the inputs to the stable crate id calculation.

lcnr · 2022-02-21T06:39:49Z

compiler/rustc_trait_selection/src/traits/query/normalize.rs

-        constant.try_super_fold_with(self)
+        let constant_kind = match constant {
+            mir::ConstantKind::Ty(c) => {
+                let const_folded = c.try_super_fold_with(self)?;


this seems wrong, we should still normalize constants here, for now this should probably be

Suggested change

let const_folded = c.try_super_fold_with(self)?;

let const_folded = c.try_fold_with(self)?;

but with valtrees we have to avoid the conversion to valtrees here.

If you change this you should be able to avoid the CI issues for now

change `mir::Constant` in mir dumps this removes duplicate information and avoids printing the `stable_crate_id` in mir dumps which broke CI in rust-lang#94059 r? `@oli-obk` cc `@b-naber`

bors · 2022-02-22T15:21:53Z

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

lcnr · 2022-03-09T12:09:39Z

src/test/ui/consts/const-eval/const-eval-query-stack.rs

@@ -16,6 +16,7 @@

 #![allow(unconditional_panic)]

+


Suggested change

lcnr · 2022-03-09T12:13:10Z

src/test/mir-opt/inline/issue_78442.bar.RevealAll.diff

@@ -25,10 +25,12 @@
          _3 = &_4;                        // scope 0 at $DIR/issue-78442.rs:11:5: 11:15
          StorageLive(_5);                 // scope 0 at $DIR/issue-78442.rs:11:5: 11:17
          nop;                             // scope 0 at $DIR/issue-78442.rs:11:5: 11:17
-          _2 = <impl Fn() as Fn<()>>::call(move _3, move _5) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/issue-78442.rs:11:5: 11:17
+-         _2 = <impl Fn() as Fn<()>>::call(move _3, move _5) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/issue-78442.rs:11:5: 11:17
+         _2 = <fn() {foo} as Fn<()>>::call(move _3, move _5) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/issue-78442.rs:11:5: 11:17


bit surprised that this changed with your pr 🤔 but as its definitely a positive change, i don't necessarily care about why this changed 🤷

lcnr · 2022-03-09T12:13:30Z

one last nit, then r=me

lcnr · 2022-03-09T12:13:39Z

@bors delegate+

bors · 2022-03-09T12:13:41Z

✌️ @b-naber can now approve this pull request

bors · 2022-03-09T15:16:24Z

📌 Commit 021c3b0 has been approved by lcnr

bors · 2022-03-09T18:19:46Z

⌛ Testing commit 021c3b0 with merge 519de42530b84c9cfbd03e9a0354724ba4ab2a5f...

bors · 2022-03-09T18:50:46Z

💔 Test failed - checks-actions

nikic · 2022-03-09T21:13:54Z

@bors retry

rust-log-analyzer · 2022-03-09T21:14:09Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

.......... (60/61)
.          (61/61)


/checkout/src/test/rustdoc-gui/jump-to-def-background.goml An exception occured: Failed to launch the browser process!
Inconsistency detected by ld.so: dl-tls.c: 493: _dl_allocate_tls_init: Assertion `listp->slotinfo[cnt].gen <= GL(dl_tls_generation)' failed!


TROUBLESHOOTING: https://github.com/puppeteer/puppeteer/blob/master/docs/troubleshooting.md
== STACKTRACE ==
Error
Error
    at innerRunTestCode (/node-v14.4.0-linux-x64/lib/node_modules/browser-ui-test/src/index.js:468:16)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
/checkout/src/test/rustdoc-gui/mobile.goml mobile... FAILED
[ERROR] (line 27) Error: Evaluation failed: assert didn't fail: for command `compare-elements-position-near-false: ("#preferred-light-theme .setting-name", "#preferred-light-theme .choice", {"y": 16})`
Build completed unsuccessfully in 0:00:17

…tion, r=lcnr Treat constant values as mir::ConstantKind::Val Another step that is necessary for the introduction of Valtrees: we don't want to treat `ty::Const` instances of kind `ty::ConstKind::Value` as `mir::ConstantKind::Ty` anymore. r? `@oli-obk`

bors · 2022-03-10T05:54:08Z

⌛ Testing commit 021c3b0 with merge d7b282b...

bors · 2022-03-10T08:35:17Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing d7b282b to master...

rust-timer · 2022-03-10T10:04:55Z

Finished benchmarking commit (d7b282b): comparison url.

Summary: This benchmark run shows 92 relevant improvements 🎉 but 6 relevant regressions 😿 to instruction counts.

Arithmetic mean of relevant regressions: 1.3%
Arithmetic mean of relevant improvements: -1.0%
Arithmetic mean of all relevant changes: -0.9%
Largest improvement in instruction counts: -6.6% on full builds of ctfe-stress-4 opt
Largest regression in instruction counts: 1.6% on full builds of keccak check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

lcnr · 2022-03-10T10:44:37Z

ConstantKind::Val doesn't need interning and we have less indirection with that variant 🤷 while i didn't expect such an improvement, i guess that's fine 😆

rylev · 2022-03-15T13:08:10Z

Since the regressions are all in secondary benchmarks and relatively small, I think it's fine if we consider this to be an improvement rather than a mixed result.

@rustbot label: +perf-regression-triaged

…li-obk Use mir constant in thir instead of ty::Const This is blocked on rust-lang#94059 (does include its changes, the first two commits in this PR correspond to those changes) and rust-lang#93800 being reinstated (which had to be reverted). Mainly opening since `@lcnr` offered to give some feedback and maybe also for a perf-run (if necessary). This currently contains a lot of duplication since some of the logic of `ty::Const` had to be copied to `mir::ConstantKind`, but with the introduction of valtrees a lot of that functionality will disappear from `ty::Const`. Only the last commit contains changes that need to be reviewed here. Did leave some `FIXME` comments regarding future implementation decisions and some things that might be incorrectly implemented. r? `@oli-obk`

Use mir constant in thir instead of ty::Const This is blocked on rust-lang/rust#94059 (does include its changes, the first two commits in this PR correspond to those changes) and rust-lang/rust#93800 being reinstated (which had to be reverted). Mainly opening since `@lcnr` offered to give some feedback and maybe also for a perf-run (if necessary). This currently contains a lot of duplication since some of the logic of `ty::Const` had to be copied to `mir::ConstantKind`, but with the introduction of valtrees a lot of that functionality will disappear from `ty::Const`. Only the last commit contains changes that need to be reviewed here. Did leave some `FIXME` comments regarding future implementation decisions and some things that might be incorrectly implemented. r? `@oli-obk`

rust-highfive assigned oli-obk Feb 16, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 16, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2022

This comment has been minimized.

Sign in to view

oli-obk reviewed Feb 16, 2022

View reviewed changes

b-naber force-pushed the constantkind-val-transformation branch from b76a6d3 to a4feb9a Compare February 18, 2022 15:29

oli-obk reviewed Feb 19, 2022

View reviewed changes

lcnr reviewed Feb 21, 2022

View reviewed changes

lcnr mentioned this pull request Feb 21, 2022

change mir::Constant in mir dumps #94209

Merged

b-naber mentioned this pull request Feb 22, 2022

Use mir constant in thir instead of ty::Const #94255

Merged

oli-obk 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 Mar 2, 2022

b-naber force-pushed the constantkind-val-transformation branch from a4feb9a to b6608af Compare March 8, 2022 19:43

This comment has been minimized.

Sign in to view

b-naber force-pushed the constantkind-val-transformation branch from b6608af to 0a82066 Compare March 9, 2022 09:14

b-naber added 3 commits March 9, 2022 10:52

treat all mir::Constant values as ConstantKind::Val

40e4bd2

bless tests

8a811a1

normalization change and rebase

26fe550

b-naber force-pushed the constantkind-val-transformation branch 2 times, most recently from 0d58cd0 to 2fa5ed2 Compare March 9, 2022 11:57

lcnr reviewed Mar 9, 2022

View reviewed changes

manually bless 32-bit stderr

18bb2dd

b-naber force-pushed the constantkind-val-transformation branch from 2fa5ed2 to 18bb2dd Compare March 9, 2022 12:41

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 9, 2022

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, 2022

Dylan-DPC mentioned this pull request Mar 9, 2022

Rollup of 16 pull requests #94784

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 10, 2022

bors merged commit d7b282b into rust-lang:master Mar 10, 2022

rustbot added this to the 1.61.0 milestone Mar 10, 2022

bors mentioned this pull request Mar 10, 2022

fix let _: ty = expr subtyping error #93856

Closed

scottmcm mentioned this pull request Mar 10, 2022

mir-opt: Replace clone on primitives with copy #94276

Merged

rustbot added the perf-regression Performance regression. label Mar 10, 2022

rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Treat constant values as mir::ConstantKind::Val #94059

Treat constant values as mir::ConstantKind::Val #94059

b-naber commented Feb 16, 2022

This comment has been minimized.

oli-obk Feb 16, 2022

b-naber Feb 18, 2022

b-naber commented Feb 18, 2022

oli-obk Feb 19, 2022

lcnr Feb 21, 2022

bjorn3 Feb 22, 2022

lcnr Feb 21, 2022

bors commented Feb 22, 2022

This comment has been minimized.

lcnr Mar 9, 2022

lcnr Mar 9, 2022

lcnr commented Mar 9, 2022

lcnr commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

nikic commented Mar 9, 2022

rust-log-analyzer commented Mar 9, 2022

bors commented Mar 10, 2022

bors commented Mar 10, 2022

rust-timer commented Mar 10, 2022

lcnr commented Mar 10, 2022

rylev commented Mar 15, 2022

	let const_folded = c.try_super_fold_with(self)?;
	let const_folded = c.try_fold_with(self)?;

Treat constant values as mir::ConstantKind::Val #94059

Treat constant values as mir::ConstantKind::Val #94059

Conversation

b-naber commented Feb 16, 2022

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-naber commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Feb 22, 2022

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Mar 9, 2022

lcnr commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

bors commented Mar 9, 2022

nikic commented Mar 9, 2022

rust-log-analyzer commented Mar 9, 2022

bors commented Mar 10, 2022

bors commented Mar 10, 2022

rust-timer commented Mar 10, 2022

lcnr commented Mar 10, 2022

rylev commented Mar 15, 2022