normalize mir::Constant differently from ty::Const in preparation for valtrees #83207

oli-obk · 2021-03-16T16:52:48Z

Valtrees are unable to represent many kind of constant values (this is on purpose). For constants that are used at runtime, we do not need a valtree representation and can thus use a different form of evaluation. In order to make this explicit and less fragile, I added a fold_constant method to TypeFolder and implemented it for normalization. Normalization can now, when it wants to eagerly evaluate a constant, normalize mir::Constant directly into a mir::ConstantKind::Val instead of relying on the ty::Const evaluation.

In the future we can get rid of the ty::Const in there entirely and add our own Unevaluated variant to mir::ConstantKind. This would allow us to remove the promoted field from ty::ConstKind::Unevaluated, as promoteds can never occur in the type system.

cc @rust-lang/wg-const-eval

r? @lcnr

rust-highfive · 2021-03-16T16:52:51Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

compiler/rustc_mir/src/interpret/operand.rs

compiler/rustc_middle/src/ty/normalize_erasing_regions.rs

compiler/rustc_trait_selection/src/traits/project.rs

RalfJung · 2021-03-16T22:14:41Z

I don't really get what this is about.^^ Normalization is not an area of the compiler that I ever had much contact with.

What I would have expected to happen is that normalization only occurs on constants that arise in types. But then why is mir::Constant relevant at all? I thought that type is for constants that occur in code... as in, it only exists as the type for wgat is stored in an Operand::Constant.

oli-obk · 2021-03-17T11:16:10Z

What I would have expected to happen is that normalization only occurs on constants that arise in types. But then why is mir::Constant relevant at all? I thought that type is for constants that occur in code... as in, it only exists as the type for wgat is stored in an Operand::Constant.

I did try not eagerly evaluating mir::Constant during normalization, but that causes cycle errors for const_evaluatable_checked tests, so I opted to normalize and revisit in the future.

Normalization is performed on MIR data structures when monomorphizing said data structures. Since constants that occur in types have MIR bodies for getting evaluated, there is some link between the two.

oli-obk · 2021-03-17T16:40:19Z

All review comments have been addressed

RalfJung · 2021-03-17T17:36:52Z

I did try not eagerly evaluating mir::Constant during normalization, but that causes cycle errors for const_evaluatable_checked tests, so I opted to normalize and revisit in the future.

Normalization is performed on MIR data structures when monomorphizing said data structures. Since constants that occur in types have MIR bodies for getting evaluated, there is some link between the two.

Okay... no idea what this entails but I'll trust it makes sense. :)

oli-obk · 2021-03-19T18:10:06Z

@bors try @rust-timer queue

rust-timer · 2021-03-19T18:10:08Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-03-19T18:10:14Z

⌛ Trying commit 0c73c0f99a34797048663ed97f23bef7b789ca1f with merge 91aa315e65b19839dca8da5a8cca9ff37c5614b3...

bors · 2021-03-19T19:10:40Z

☀️ Try build successful - checks-actions
Build commit: 91aa315e65b19839dca8da5a8cca9ff37c5614b3 (91aa315e65b19839dca8da5a8cca9ff37c5614b3)

rust-timer · 2021-03-19T19:10:42Z

Queued 91aa315e65b19839dca8da5a8cca9ff37c5614b3 with parent b97fd3e, future comparison URL.

rust-timer · 2021-03-19T23:45:02Z

Finished benchmarking try commit (91aa315e65b19839dca8da5a8cca9ff37c5614b3): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

oli-obk · 2021-03-20T13:26:56Z

The stress tests regressed. Looking at the details, we have 40% fewer eval_to_const_value_raw query invocations and 80% fewer normalize_generic_arg_after_erasing_regions invocations, but a 4% increase in time taken for eval_to_allocation_raw.

oli-obk · 2021-03-22T12:58:25Z

@bors try @rust-timer queue

rust-timer · 2021-03-22T12:58:27Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-timer · 2021-04-01T12:01:42Z

Finished benchmarking try commit (7409b4198347a26b96fdc2e2066c937a2af64972): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

bors · 2021-04-01T12:23:49Z

☀️ Try build successful - checks-actions
Build commit: e47c8d1017e71dbf47d9d88f3f9e8c008248c9e7 (e47c8d1017e71dbf47d9d88f3f9e8c008248c9e7)

oli-obk · 2021-04-01T13:02:04Z

Local perf run (with the latest try build, not the same as the perf run above):

--------------------------------------------------------------------------------
           Ir 
--------------------------------------------------------------------------------
 -818,318,722  PROGRAM TOTALS

--------------------------------------------------------------------------------
            Ir  file:function
--------------------------------------------------------------------------------
-3,373,631,810  ???:rustc_mir::interpret::eval_context::InterpCx<M>::layout_of_local
 2,503,347,014  ???:rustc_mir::interpret::place::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_place
 1,221,597,116  ???:rustc_middle::ty::normalize_erasing_regions::<impl rustc_middle::ty::context::TyCtxt>::normalize_erasing_regions
-1,156,828,134  ???:rustc_middle::ty::instance::Instance::subst_mir_and_normalize_erasing_regions
-1,149,246,226  ???:rustc_middle::ty::normalize_erasing_regions::<impl rustc_middle::ty::context::TyCtxt>::subst_and_normalize_erasing_regions
  -399,081,153  ???:<rustc_middle::ty::normalize_erasing_regions::NormalizeAfterErasingRegionsFolder as rustc_middle::ty::fold::TypeFolder>::fold_const
   351,271,730  ???:hashbrown::map::RawEntryBuilder<K,V,S,A>::from_key_hashed_nocheck
   317,144,070  ???:rustc_mir::interpret::operand::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_operand
   309,330,160  ???:core::cmp::impls::<impl core::cmp::PartialEq<&B> for &A>::eq
   220,211,940  ???:<rustc_middle::ty::consts::kind::ConstKind as core::hash::Hash>::hash
   218,956,888  ???:rustc_mir::interpret::operand::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::access_local
    76,021,528  ???:rustc_mir::interpret::eval_context::InterpCx<M>::subst_from_current_frame_and_normalize_erasing_regions
    68,472,035  ???:rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_rvalue_into_place
   -26,214,318  ???:<core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::try_fold
    15,728,640  ???:rustc_middle::ty::<impl rustc_data_structures::tagged_ptr::Tag for rustc_middle::traits::Reveal>::from_usize
    -8,912,866  ???:<rustc_middle::ty::layout::LayoutCx<rustc_middle::ty::query::TyCtxtAt> as rustc_target::abi::LayoutOf>::layout_of
    -2,621,721  ???:rustc_mir::interpret::eval_context::InterpCx<M>::load_mir
    -2,621,445  ???:rustc_mir::interpret::terminator::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_fn_call
    -1,311,308  ???:rustc_middle::ty::instance::Instance::resolve_opt_const_arg

oli-obk · 2021-04-01T13:02:56Z

@bors r=lcnr (changes to previous approval are: some useless commits removed, some inline attributes added).

bors · 2021-04-01T13:02:58Z

📌 Commit c6676db has been approved by lcnr

bors · 2021-04-01T14:52:13Z

⌛ Testing commit c6676db with merge 18b838f728bf7578c7a5821d4e563ac758f60f5b...

rust-log-analyzer · 2021-04-01T15:41:33Z

A job failed! Check out the build log: (web) (plain)

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

oli-obk · 2021-04-01T16:03:13Z

@bors retry

bors · 2021-04-01T19:02:42Z

⌛ Testing commit c6676db with merge 98107010e61a1adef6724de613e2daf33ca41c9e...

bors · 2021-04-01T23:17:42Z

💥 Test timed out

oli-obk · 2021-04-02T07:16:00Z

@bors retry

bors · 2021-04-02T10:28:16Z

⌛ Testing commit c6676db with merge 0978a9e...

bors · 2021-04-02T12:51:35Z

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

oli-obk added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Mar 16, 2021

rust-highfive assigned lcnr Mar 16, 2021

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

This comment has been minimized.

Sign in to view

lcnr reviewed Mar 16, 2021

View reviewed changes

oli-obk force-pushed the valtree2 branch from 0a1ea30 to 1117c92 Compare March 17, 2021 16:39

oli-obk mentioned this pull request Mar 17, 2021

Use ValTree in all type level constants #83234

Closed

This comment has been minimized.

Sign in to view

oli-obk force-pushed the valtree2 branch from 1117c92 to 0c73c0f Compare March 18, 2021 10:19

oli-obk removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Mar 19, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 19, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 19, 2021

oli-obk force-pushed the valtree2 branch from 0c73c0f to 090bb39 Compare March 22, 2021 12:58

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 22, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Apr 1, 2021

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 Apr 1, 2021

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 Apr 2, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 2, 2021

bors merged commit 0978a9e into rust-lang:master Apr 2, 2021

rustbot added this to the 1.53.0 milestone Apr 2, 2021

oli-obk deleted the valtree2 branch April 2, 2021 13:08

lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021

lcnr mentioned this pull request Dec 11, 2021

Tracking issue for const generics (RFC 2000) #44580

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

normalize mir::Constant differently from ty::Const in preparation for valtrees #83207

normalize mir::Constant differently from ty::Const in preparation for valtrees #83207

oli-obk commented Mar 16, 2021 •

edited

rust-highfive commented Mar 16, 2021

This comment has been minimized.

RalfJung commented Mar 16, 2021

oli-obk commented Mar 17, 2021

oli-obk commented Mar 17, 2021

This comment has been minimized.

RalfJung commented Mar 17, 2021

oli-obk commented Mar 19, 2021

rust-timer commented Mar 19, 2021

bors commented Mar 19, 2021

bors commented Mar 19, 2021

rust-timer commented Mar 19, 2021

rust-timer commented Mar 19, 2021

oli-obk commented Mar 20, 2021

oli-obk commented Mar 22, 2021

rust-timer commented Mar 22, 2021

rust-timer commented Apr 1, 2021

bors commented Apr 1, 2021

oli-obk commented Apr 1, 2021

oli-obk commented Apr 1, 2021

bors commented Apr 1, 2021

bors commented Apr 1, 2021

rust-log-analyzer commented Apr 1, 2021

oli-obk commented Apr 1, 2021

bors commented Apr 1, 2021

bors commented Apr 1, 2021

oli-obk commented Apr 2, 2021

bors commented Apr 2, 2021

bors commented Apr 2, 2021

normalize mir::Constant differently from ty::Const in preparation for valtrees #83207

normalize mir::Constant differently from ty::Const in preparation for valtrees #83207

Conversation

oli-obk commented Mar 16, 2021 • edited

rust-highfive commented Mar 16, 2021

This comment has been minimized.

RalfJung commented Mar 16, 2021

oli-obk commented Mar 17, 2021

oli-obk commented Mar 17, 2021

This comment has been minimized.

RalfJung commented Mar 17, 2021

oli-obk commented Mar 19, 2021

rust-timer commented Mar 19, 2021

bors commented Mar 19, 2021

bors commented Mar 19, 2021

rust-timer commented Mar 19, 2021

rust-timer commented Mar 19, 2021

oli-obk commented Mar 20, 2021

oli-obk commented Mar 22, 2021

rust-timer commented Mar 22, 2021

rust-timer commented Apr 1, 2021

bors commented Apr 1, 2021

oli-obk commented Apr 1, 2021

oli-obk commented Apr 1, 2021

bors commented Apr 1, 2021

bors commented Apr 1, 2021

rust-log-analyzer commented Apr 1, 2021

oli-obk commented Apr 1, 2021

bors commented Apr 1, 2021

bors commented Apr 1, 2021

oli-obk commented Apr 2, 2021

bors commented Apr 2, 2021

bors commented Apr 2, 2021

oli-obk commented Mar 16, 2021 •

edited