Treat static refs as `mir::ConstantKind::Val` #93800

b-naber · 2022-02-09T10:50:30Z

With the upcoming introduction of Valtrees we want to treat more values as mir::ConstantKind::Val directly.

r? @lcnr

cc @oli-obk

compiler/rustc_mir_build/src/build/expr/as_constant.rs

src/test/mir-opt/const_allocation.main.ConstProp.after.64bit.mir

oli-obk · 2022-02-09T16:19:28Z

lgtm

r=me with 32 bit mir dumps blessed (this is why CI is failing)

b-naber · 2022-02-09T17:36:02Z

r=me with 32 bit mir dumps blessed (this is why CI is failing)

Not sure how to do that. I'm on Mac OS and tried a complete rebuild with: ./x.py test src/test/mir-opt --bless --target i686-apple-darwin, but that fails to build the std with a linker error?!

oli-obk · 2022-02-09T20:34:49Z

I can try pushing to your branch tomorrow. I wish we could just tell bors to bless or have a bot that scrapes the blessed files from CI

b-naber · 2022-02-14T16:36:00Z

@oli-obk This is ready.

oli-obk · 2022-02-14T17:08:47Z

-.- I need a a way to set reminders for myself when I write such comments.

Thanks for going the hard way and copying from CI

@bors r+

bors · 2022-02-14T17:08:48Z

📌 Commit 5e36b5daa5aa4960d8198bf0d6d6926051f47201 has been approved by oli-obk

b-naber · 2022-02-14T17:21:37Z

-.- I need a a way to set reminders for myself when I write such comments.

No worries.

bors · 2022-02-15T15:09:25Z

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

…uction

oli-obk · 2022-02-15T20:33:39Z

@bors r+

bors · 2022-02-16T03:03:06Z

⌛ Testing commit db019f2 with merge a240ccd...

bors · 2022-02-16T06:45:01Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing a240ccd to master...

rust-timer · 2022-02-16T08:16:35Z

Finished benchmarking commit (a240ccd): comparison url.

Summary: This benchmark run did not return any relevant results. 11 results were found to be statistically significant but too small to be relevant.

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

@rustbot label: -perf-regression

…ir-val, r=oli-obk" This reverts commit a240ccd, reversing changes made to 393fdc1. This PR was likely responsible for a relatively large regression in dist-x86_64-msvc-alt builder times, from approximately 1.7 to 2.8 hours, bringing that builder into the pool of the slowest builders we currently have. This seems to be limited to the alt builder due to needing parallel-compiler enabled, likely leading to slow LLVM compilation for some reason.

Revert rust-lang#93800, fixing CI time regression This reverts commit a240ccd (merge commit of rust-lang#93800), reversing changes made to 393fdc1. This PR was likely responsible for a relatively large regression in dist-x86_64-msvc-alt builder times, from approximately 1.7 to 2.8 hours, bringing that builder into the pool of the slowest builders we currently have. This seems to be limited to the alt builder due to needing parallel-compiler enabled, likely leading to slow LLVM compilation for some reason. See some investigation in [this Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/msvc.28.3F.29.20builders.20running.20much.20slower). cc `@lcnr` `@oli-obk` `@b-naber` (per original PRs review/author) We can re-apply this PR once the regression is fixed, but it is sufficiently large that I don't think keeping this on master is viable in the meantime unless there's a very strong case to be made for it. Alternatively, we can disable that builder (it's not critical since it's an alt build), but that obviously carries its own costs.

…r=oli-obk Treat static refs as `mir::ConstantKind::Val` With the upcoming introduction of Valtrees we want to treat more values as `mir::ConstantKind::Val` directly. r? `@lcnr` cc `@oli-obk`

rust-log-analyzer · 2022-02-25T20:41:54Z

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

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

Some tests failed in compiletest suite=mir-opt mode=mir-opt host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
..................................i................................
failures:

---- [mir-opt] mir-opt/const-promotion-extern-static.rs stdout ----
1 - // MIR for `BAR` before PromoteTemps
2 + // MIR for `BAR` after PromoteTemps
+   
+   
4   static mut BAR: *const &i32 = {
5       let mut _0: *const &i32;             // return place in scope 0 at $DIR/const-promotion-extern-static.rs:9:17: 9:28
6       let mut _1: &[&i32];                 // in scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:44

9       let mut _4: &i32;                    // in scope 0 at $DIR/const-promotion-extern-static.rs:9:32: 9:34
10       let _5: &i32;                        // in scope 0 at $DIR/const-promotion-extern-static.rs:9:33: 9:34
11 +     let mut _6: &[&i32; 1];              // in scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:44
+   
13       bb0: {
13       bb0: {
14           StorageLive(_1);                 // scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:44
15           StorageLive(_2);                 // scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:44

35                                            // + span: $DIR/const-promotion-extern-static.rs:9:36: 9:42
36                                            // + literal: Const { ty: for<'r> fn(&'r [&i32]) -> *const &i32 {core::slice::<impl [&i32]>::as_ptr}, val: Value(Scalar(<ZST>)) }
- 
+   
39       bb1: {
39       bb1: {
40 -         StorageDead(_5);                 // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44
41 -         StorageDead(_3);                 // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44

42           StorageDead(_1);                 // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44
43           return;                          // scope 0 at $DIR/const-promotion-extern-static.rs:9:1: 9:45
- 
+   
+   
46       bb2 (cleanup): {
47           resume;                          // scope 0 at $DIR/const-promotion-extern-static.rs:9:1: 9:45

49 - }
- -
+ - 
+ - 
51 - alloc1 (static: Y, size: 4, align: 4) {
52 -     2a 00 00 00                                     │ *...

- 
+   
55 
55 

thread '[mir-opt] mir-opt/const-promotion-extern-static.rs' panicked at 'Actual MIR output differs from expected MIR output /checkout/src/test/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff', src/tools/compiletest/src/runtest.rs:3386:25


failures:
    [mir-opt] mir-opt/const-promotion-extern-static.rs

Reinstate rust-lang#93800 rust-lang#93800 caused a regression in an alt builder with parallel enabled. rust-lang#94205 reverted that PR because of the regression. For an unknown reason the regression has disappeared, so we reinstate the changes in rust-lang#93800 here. r? `@Mark-Simulacrum`

…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 lcnr Feb 9, 2022

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

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

b-naber changed the title ~~Treat static initialisers as mir::ConstantKind::Val~~ Treat static refs as mir::ConstantKind::Val Feb 9, 2022

oli-obk requested changes Feb 9, 2022

View reviewed changes

compiler/rustc_mir_build/src/build/expr/as_constant.rs Outdated Show resolved Hide resolved

src/test/mir-opt/const_allocation.main.ConstProp.after.64bit.mir Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

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

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

b-naber added 6 commits February 15, 2022 21:10

use ConstantKind::Val in StaticRef

5e0fab6

treat mir::ConstantKind::Val correctly in check_static_ptr

c612ef8

bless tests

8092b90

use mir::Visitor when collecting alloc_ids in pretty printing

22d6204

bless mir-opt tests

54ff25e

use AllocId and Ty in ExprKind::StaticRef and delay ConstValue constr…

fff06e5

…uction

b-naber force-pushed the static-initializers-mir-val branch from 5e36b5d to e9c0763 Compare February 15, 2022 20:25

This comment has been minimized.

Sign in to view

b-naber force-pushed the static-initializers-mir-val branch 2 times, most recently from b99f38c to b8a065d Compare February 15, 2022 20:31

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 16, 2022

bors merged commit a240ccd into rust-lang:master Feb 16, 2022

rustbot added this to the 1.60.0 milestone Feb 16, 2022

Mark-Simulacrum mentioned this pull request Feb 20, 2022

[DO NOT MERGE] CI time regression fixing experiments #94195

Closed

Mark-Simulacrum mentioned this pull request Feb 21, 2022

Revert #93800, fixing CI time regression #94205

Merged

b-naber mentioned this pull request Feb 22, 2022

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

Merged

b-naber mentioned this pull request Mar 4, 2022

[EXPERIMENT] Figure out which commit caused regression in parallel alt builder #94604

Closed

b-naber mentioned this pull request Mar 7, 2022

Reinstate #93800 #94702

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Treat static refs as `mir::ConstantKind::Val` #93800

Treat static refs as `mir::ConstantKind::Val` #93800

b-naber commented Feb 9, 2022 •

edited

Loading

This comment has been minimized.

oli-obk commented Feb 9, 2022

This comment has been minimized.

b-naber commented Feb 9, 2022

oli-obk commented Feb 9, 2022

b-naber commented Feb 14, 2022

oli-obk commented Feb 14, 2022

bors commented Feb 14, 2022

b-naber commented Feb 14, 2022

bors commented Feb 15, 2022

This comment has been minimized.

oli-obk commented Feb 15, 2022

bors commented Feb 16, 2022

bors commented Feb 16, 2022

rust-timer commented Feb 16, 2022

rust-log-analyzer commented Feb 25, 2022

Treat static refs as mir::ConstantKind::Val #93800

Treat static refs as mir::ConstantKind::Val #93800

Conversation

b-naber commented Feb 9, 2022 • edited Loading

This comment has been minimized.

oli-obk commented Feb 9, 2022

This comment has been minimized.

b-naber commented Feb 9, 2022

oli-obk commented Feb 9, 2022

b-naber commented Feb 14, 2022

oli-obk commented Feb 14, 2022

bors commented Feb 14, 2022

b-naber commented Feb 14, 2022

bors commented Feb 15, 2022

This comment has been minimized.

oli-obk commented Feb 15, 2022

bors commented Feb 16, 2022

bors commented Feb 16, 2022

rust-timer commented Feb 16, 2022

rust-log-analyzer commented Feb 25, 2022

Treat static refs as `mir::ConstantKind::Val` #93800

Treat static refs as `mir::ConstantKind::Val` #93800

b-naber commented Feb 9, 2022 •

edited

Loading