interpret: skip deref-projection validity checks when they are not needed

[RalfJung]

View all comments

Trying to claw back the perf regression from #160012.

We could also, like, skip the entire check in const-eval when we don't care about validity. But the slowdown will also affect Miri so I want to first try what we can do without doing less UB checking.

[RalfJung]

@bors try
@rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: fd82567 (fd825675c9cb0054bc1537dbd227fc750bc2fbaa)
Base parent: 28c66af (28c66af27578a28bc8cad1c5957be4b1e2e7fb8b)

[rust-timer]

Finished benchmarking commit (fd82567): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.2%	[-10.2%, -8.6%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -2.8%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Cycles

Results (primary -0.2%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.6%, 0.6%]	4
Regressions ❌ (secondary)	0.9%	[0.4%, 1.4%]	5
Improvements ✅ (primary)	-0.8%	[-1.0%, -0.5%]	5
Improvements ✅ (secondary)	-1.5%	[-3.4%, -0.4%]	6
All ❌✅ (primary)	-0.2%	[-1.0%, 0.6%]	9

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 490.376s -> 490.585s (0.04%)
Artifact size: 390.46 MiB -> 390.48 MiB (0.01%)

[RalfJung]

Looks like it helps, but we need to skip all the checks to regain the original performance. And I guess for Miri this just will be an actual ~5% slowdown...

@bors try
@rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 857e16e (857e16ea6e812013b38eea7b5ecbc5162496574b)
Base parent: 65dd30f (65dd30fb9e882a7e8f0be10caca62936db2a98b8)

[rust-timer]

Finished benchmarking commit (857e16e): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-13.1%	[-14.2%, -12.2%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.5%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.6%	[0.9%, 4.3%]	2
Regressions ❌ (secondary)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-0.6%	[-0.9%, -0.4%]	15
All ❌✅ (primary)	1.5%	[-0.8%, 4.3%]	3

Cycles

Results (primary 0.6%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.2%	[0.5%, 3.1%]	6
Regressions ❌ (secondary)	1.7%	[0.5%, 3.9%]	7
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.5%]	3
Improvements ✅ (secondary)	-2.3%	[-4.1%, -0.4%]	4
All ❌✅ (primary)	0.6%	[-0.6%, 3.1%]	9

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 491.898s -> 488.826s (-0.62%)
Artifact size: 391.08 MiB -> 390.47 MiB (-0.16%)

[RalfJung]

r? @oli-obk

[rustbot]

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @oli-obk, @lcnr

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

[oli-obk]

@bors r+

Sad about the early UB errors. Maybe some day we'll just rerun with all checks when the final value has an error?

View changes since this review

[rust-bors]

📌 Commit 75505d9 has been approved by oli-obk

It is now in the queue for this repository.

[Kobzol]

Marking as rollup=iffy, because this only affects one benchmark, and we have a lot of rollup=never PRs in the queue.

@bors rollup=iffy

[JonathanBrouwer]

Verifying that actual perf results after merge match expected results
@rust-timer build 6e167bd

[rust-timer]

Finished benchmarking commit (6e167bd): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-12.8%	[-13.9%, -11.9%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.2%, secondary -0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.5%, 0.8%]	7
Regressions ❌ (secondary)	0.5%	[0.4%, 0.7%]	5
Improvements ✅ (primary)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-6.0%	[-6.0%, -6.0%]	1
All ❌✅ (primary)	0.2%	[-2.3%, 0.8%]	8

Cycles

Results (primary -0.3%, secondary -0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.4%, 1.3%]	4
Regressions ❌ (secondary)	1.0%	[0.4%, 2.9%]	6
Improvements ✅ (primary)	-0.8%	[-1.6%, -0.5%]	7
Improvements ✅ (secondary)	-0.8%	[-1.6%, -0.4%]	8
All ❌✅ (primary)	-0.3%	[-1.6%, 1.3%]	11

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.577s -> 491.533s (0.40%)
Artifact size: 390.24 MiB -> 390.55 MiB (0.08%)