GVN: Don't assume nested shared references are read-only.

[dianqk]

Fixes #155884.

#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.

cc @RalfJung @saethlin

r? @cjgillot

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[dianqk]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 0eeeb18 (0eeeb181f41359e276c1dae9eec938e457300cec, parent: d595fce01043347bf7f80e85b76dcc41b59a3e6e)

[rust-timer]

Finished benchmarking commit (0eeeb18): 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
@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.3%	[0.1%, 1.0%]	12
Regressions ❌ (secondary)	0.3%	[0.0%, 0.5%]	36
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.2%]	12
Improvements ✅ (secondary)	-0.3%	[-1.0%, -0.0%]	12
All ❌✅ (primary)	0.0%	[-0.3%, 1.0%]	24

Max RSS (memory usage)

Results (primary 3.6%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[1.8%, 10.4%]	6
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.9%	[-3.9%, -3.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.6%	[-3.9%, 10.4%]	7

Cycles

Results (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)	7.2%	[5.0%, 10.8%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.7%	[-6.2%, -2.4%]	7
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.0%, secondary 0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.6%]	19
Regressions ❌ (secondary)	0.1%	[0.0%, 0.6%]	16
Improvements ✅ (primary)	-0.4%	[-1.1%, -0.1%]	8
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-0.0%	[-1.1%, 0.6%]	27

Bootstrap: 509.701s -> 512.229s (0.50%)
Artifact size: 400.67 MiB -> 398.49 MiB (-0.55%)

[RalfJung]

This isn't quite grammatical. Maybe you mean "do not reason about ..."? But even then I'm a bit confused about what the comment is trying to say.

This should definitely mention that nested shared references are not read-only and have a link to the issue.

View changes since the review

[RalfJung]

Suggested change

	// Unifying them can violate Stacked Borrows or Tree Borrows.
	// Furthermore, unifying them can also violate Stacked Borrows or Tree Borrows.

View changes since the review