interpret: remove support for uninitialized scalars

[RalfJung]

With Miri no longer supporting -Zmiri-allow-uninit-numbers, we no longer need to support storing uninit data in a Scalar. We anyway already only use this representation for types with initialized Scalar layout (and we have to, due to partial initialization), so let's get rid of the ScalarMaybeUninit type entirely.

I tried to stage this into meaningful commits, but the one that changes read_immediate to always trigger UB on uninit is the largest chunk of the PR and I don't see how it could be subdivided.

Fixes rust-lang/miri#2187
r? @oli-obk

[rustbot]

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

[RalfJung]

Given #99923, we probably want to crater this one.
@bors try

[bors]

⌛ Trying commit c006f64b0d9e4619f0f5983be5c249e99d0b5b70 with merge a8425075041d4b9af9bf32e2a267dfb9db2cfffb...

[bors]

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

[RalfJung]

@craterbot check

[craterbot]

👌 Experiment pr-100043 created and queued.
🤖 Automatically detected try build a8425075041d4b9af9bf32e2a267dfb9db2cfffb
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-100043 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-100043 is completed!
📊 686 regressed and 1 fixed (240377 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[RalfJung]

Most of these failures are due to "Failed to get rustc version" in the libc build script... what is going on there? That can't be this PR. Out of the 10 I have checked so far, all 10 show that error. Is there some way that we don't have to check >600 regressions manually for whether they are all the same spurious failure...?

[RalfJung]

I downloaded the 'regressed' logs, removed the spurious libc things, spurious syn build failures ("trait X does not implement Y"), and some strange spurious "file not found for module", and went through the rest by hand, and they are all spurious.

[RalfJung]

Just to help diagnose the crater issue:
@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-100043/retry-regressed-list.txt

[craterbot]

👌 Experiment pr-100043-1 created and queued.
🤖 Automatically detected try build a8425075041d4b9af9bf32e2a267dfb9db2cfffb
⚠️ Try build based on commit c006f64b0d9e4619f0f5983be5c249e99d0b5b70, but latest commit is 29193dd3cc26f169feed5392627feb082c7f349e. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[Mark-Simulacrum]

@craterbot p=1

[craterbot]

📝 Configuration of the pr-100043-1 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-100043-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-100043-1 is completed!
📊 15 regressed and 0 fixed (686 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[RalfJung]

@Dylan-DPC so far all I can see there are Miri build failures, which are expected and allowed.

[RalfJung]

@bors r+ rollup=never

[bors]

📌 Commit 62b6a8b has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 62b6a8b with merge 2b443a8...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 2b443a8 to master...

[rust-highfive]

📣 Toolstate changed by #100043!

Tested on commit 2b443a8.
Direct link to PR: #100043

💔 miri on windows: test-pass → build-fail (cc @RalfJung @oli-obk).
💔 miri on linux: test-pass → build-fail (cc @RalfJung @oli-obk).

[rust-timer]

Finished benchmarking commit (2b443a8): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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
cc @rust-lang/wg-compiler-performance

Instruction count

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

	mean1	range	count2
Regressions ❌ (primary)	0.8%	[0.6%, 0.9%]	2
Regressions ❌ (secondary)	1.3%	[0.3%, 3.2%]	18
Improvements ✅ (primary)	-0.6%	[-0.7%, -0.5%]	6
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-0.7%, 0.9%]	8

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.

	mean1	range	count2
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	5.4%	[2.1%, 8.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[3.1%, 3.1%]	1

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.

	mean1	range	count2
Regressions ❌ (primary)	2.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	4.5%	[2.7%, 6.3%]	9
Improvements ✅ (primary)	-2.5%	[-2.5%, -2.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-2.5%, 2.3%]	2

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[RalfJung]

Some improvements, some regressions, does that count as them being canceled out so overall we are good?^^

The code became simpler than before so I don't quite understand why this would slow anything down.

[rylev]

Looks like html5ever-0.26.0 has become noisy and the improvements here are making up for regressions seen in the last PR (and thus unlikely to be due to the changes here).

The regressions in secondary benchmarks seem to be more real though. Looks like the most impacted query is eval_to_allocation_raw. Seems possible that that might indeed be impacted by this change (just going off the usage of eval_to_allocation_raw in const eval)?

Here's a run of cachegrind diff on one of the regressions:

 9,371,649  ???:do_rallocx
 8,716,171  ???:_rjem_je_arena_ralloc
 6,029,220  ???:_rjem_je_arena_ralloc_no_move
-3,604,458  ???:<hashbrown::set::HashSet<rustc_middle::mir::interpret::AllocId, core::hash::BuildHasherDefault<rustc_hash::FxHasher>> as core::iter::traits::collect::Extend<rustc_middle::mir::interpret::AllocId>>::extend::<core::iter::adapters::map::Map<core::slice::iter::Iter<(rustc_target::abi::Size, rustc_middle::mir::interpret::AllocId)>, rustc_const_eval::interpret::intern::intern_shallow<rustc_const_eval::const_eval::machine::CompileTimeInterpreter>::{closure
 3,604,458  ???:<hashbrown::map::HashMap<rustc_middle::mir::interpret::AllocId, (), core::hash::BuildHasherDefault<rustc_hash::FxHasher>> as core::iter::traits::collect::Extend<(rustc_middle::mir::interpret::AllocId, ())>>::extend::<core::iter::adapters::map::Map<core::iter::adapters::map::Map<core::slice::iter::Iter<(rustc_target::abi::Size, rustc_middle::mir::interpret::AllocId)>, rustc_const_eval::interpret::intern::intern_shallow<rustc_const_eval::const_eval::machine::CompileTimeInterpreter>::{closure
 3,276,750  ???:<rustc_const_eval::interpret::validity::ValidityVisitor<rustc_const_eval::const_eval::machine::CompileTimeInterpreter>>::check_safe_pointer
 2,949,075  library/core/src/fmt/mod.rs:core::fmt::write
 2,818,180  /build/glibc-eX1tMB/glibc-2.31/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:__memcpy_avx_unaligned_erms
-2,359,247  ???:<rustc_const_eval::interpret::eval_context::InterpCx<rustc_const_eval::const_eval::machine::CompileTimeInterpreter>>::ref_to_mplace

Could it be more allocations happening in the validity visitor?

[RalfJung]

Ah! Yes there is an unconditional format! that is only needed on the error path. Good catch!

[rylev]

Given the results of the fix PR being positive, I'm going to mark this as triaged.

@rustbot label: +perf-regression-triaged