Various const eval cleanups #116835

oli-obk · 2023-10-17T07:13:34Z

This pulls out the pure refactorings from #116564

r? @RalfJung

rustbot · 2023-10-17T07:13:45Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

oli-obk · 2023-10-17T07:13:48Z

@bors try @rust-timer queue

bors · 2023-10-17T07:14:58Z

⌛ Trying commit 76869b4 with merge 7f8df04...

…2, r=<try> Various const eval cleanups This pulls out the pure refactorings from rust-lang#116564 r? `@RalfJung`

compiler/rustc_const_eval/src/interpret/eval_context.rs

compiler/rustc_const_eval/src/interpret/validity.rs

bors · 2023-10-17T08:34:53Z

☀️ Try build successful - checks-actions
Build commit: 7f8df04 (7f8df045973bf36ad8828db68d0f31fbd8d172ae)

rust-timer · 2023-10-17T17:47:02Z

Finished benchmarking commit (7f8df04): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.6%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.6%	[3.6%, 3.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[-1.1%, 3.6%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.5%	[6.5%, 6.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 625.939s -> 625.396s (-0.09%)
Artifact size: 305.58 MiB -> 305.58 MiB (0.00%)

oli-obk · 2023-10-20T15:43:47Z

This is ready for review again

RalfJung · 2023-10-20T16:56:44Z

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

This is just outlining, right? These new functions only have a single call site?

RalfJung · 2023-10-20T16:59:55Z

compiler/rustc_const_eval/src/interpret/eval_context.rs

-        let param_env = if self.tcx.is_static(gid.instance.def_id()) {
-            ty::ParamEnv::reveal_all()
+        let val = if self.tcx.is_static(gid.instance.def_id()) {
+            let alloc_id = self.tcx.reserve_and_set_static_alloc(gid.instance.def_id());


Ah, clever!
Could you a comment explaining why we're doing it this way?

In particular, this doesn't actually add the assertion that eval_to_allocation_raw is never called on statics, or does it?

No, because that isn't true in this PR yet.

RalfJung · 2023-10-20T18:38:02Z

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

                // Convert to raw constant
-                Ok(ConstAlloc { alloc_id, ty: mplace.layout.ty })
-            }
+                .map(|()| ConstAlloc { alloc_id, ty: mplace.layout.ty })


Honestly I find the match more readable than using higher-order functions... but I don't care very strongly.

I force pushed away the commit that did this

…m miri

RalfJung · 2023-10-23T08:43:33Z

Thanks!
@bors r+

bors · 2023-10-23T08:43:35Z

📌 Commit 066ec12 has been approved by RalfJung

It is now in the queue for this repository.

bors · 2023-10-23T09:51:30Z

⌛ Testing commit 066ec12 with merge 27dfe1e...

…2, r=RalfJung Various const eval cleanups This pulls out the pure refactorings from rust-lang#116564 r? `@RalfJung`

bors · 2023-10-23T09:58:31Z

💔 Test failed - checks-actions

rust-log-analyzer · 2023-10-23T10:00:49Z

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 · 2023-10-23T10:06:36Z

@bors retry curl: (28) SSL/TLS connection timeout

bors · 2023-10-23T12:01:21Z

⌛ Testing commit 066ec12 with merge 8501f1c...

bors · 2023-10-23T13:56:12Z

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 8501f1c to master...

rust-timer · 2023-10-23T16:50:11Z

Finished benchmarking commit (8501f1c): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.6%, -0.5%]	5
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: missing data
Artifact size: 304.30 MiB -> 304.30 MiB (0.00%)

rustbot assigned RalfJung Oct 17, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 17, 2023

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 17, 2023

Auto merge of rust-lang#116835 - oli-obk:evaluated_static_in_metadata…

7f8df04

…2, r=<try> Various const eval cleanups This pulls out the pure refactorings from rust-lang#116564 r? `@RalfJung`

RalfJung reviewed Oct 17, 2023

View reviewed changes

compiler/rustc_const_eval/src/interpret/eval_context.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 17, 2023

View reviewed changes

compiler/rustc_const_eval/src/interpret/validity.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

oli-obk force-pushed the evaluated_static_in_metadata2 branch from 76869b4 to b88a732 Compare October 17, 2023 08:51

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 17, 2023

RalfJung reviewed Oct 20, 2023

View reviewed changes

RalfJung mentioned this pull request Oct 22, 2023

Test and assert that we do not reveal a static item's memory AllocId #116571

Closed

oli-obk added 5 commits October 23, 2023 08:35

The mutability was ignored anyway, so just check for staticness

0fce74e

Pull mplace validation logic out into an interpreter method

5784e9e

Pull out const error reporting into its own function

f85b139

Split eval_to_allocation_raw_provider

fec0b54

Ensure that eval_to_allocation_raw isn't called on static items fro…

066ec12

…m miri

oli-obk force-pushed the evaluated_static_in_metadata2 branch from b88a732 to 066ec12 Compare October 23, 2023 08:35

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 23, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 23, 2023

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 23, 2023

Auto merge of rust-lang#116835 - oli-obk:evaluated_static_in_metadata…

27dfe1e

…2, r=RalfJung Various const eval cleanups This pulls out the pure refactorings from rust-lang#116564 r? `@RalfJung`

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 Oct 23, 2023

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 Oct 23, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 23, 2023

bors merged commit 8501f1c into rust-lang:master Oct 23, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 23, 2023

oli-obk deleted the evaluated_static_in_metadata2 branch October 23, 2023 16:55

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

matthiaskrgr mentioned this pull request Apr 16, 2024

ICE: node HirId ... cannot be placed in TypeckResults with hir_owner DefId... #124022

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various const eval cleanups #116835

Various const eval cleanups #116835

oli-obk commented Oct 17, 2023

rustbot commented Oct 17, 2023

oli-obk commented Oct 17, 2023

This comment has been minimized.

bors commented Oct 17, 2023

bors commented Oct 17, 2023

This comment has been minimized.

rust-timer commented Oct 17, 2023

oli-obk commented Oct 20, 2023

RalfJung Oct 20, 2023

oli-obk Oct 20, 2023

RalfJung Oct 20, 2023

RalfJung Oct 20, 2023

oli-obk Oct 20, 2023

RalfJung Oct 20, 2023

oli-obk Oct 23, 2023

RalfJung commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

rust-log-analyzer commented Oct 23, 2023

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

rust-timer commented Oct 23, 2023

Various const eval cleanups #116835

Various const eval cleanups #116835

Conversation

oli-obk commented Oct 17, 2023

rustbot commented Oct 17, 2023

oli-obk commented Oct 17, 2023

This comment has been minimized.

bors commented Oct 17, 2023

bors commented Oct 17, 2023

This comment has been minimized.

rust-timer commented Oct 17, 2023

Overall result: ✅ improvements - no action needed

oli-obk commented Oct 20, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

rust-log-analyzer commented Oct 23, 2023

oli-obk commented Oct 23, 2023

bors commented Oct 23, 2023

bors commented Oct 23, 2023

rust-timer commented Oct 23, 2023

Overall result: ✅ improvements - no action needed