Move Option::as_slice to an always-sound implementation

[scottmcm]

This approach depends on CSE to not have any branches or selects when the guessed offset is correct -- which it always will be right now -- but to also be sound (just less efficient) if the layout algorithms change such that the guess is incorrect.

The codegen test confirms that CSE handles this as expected, leaving the optimal codegen.

cc JakobDegen #108545

[rustbot]

r? @m-ou-se

(rustbot has picked a reviewer for you, use r? to override)

[scottmcm]

Output of the codegen test on my machine, in case it helps a reviewer:

; Function Attrs: mustprogress nofree nosync nounwind willreturn uwtable
define { ptr, i64 } @u64_opt_as_slice(ptr noalias noundef readonly align 8 dereferenceable(16) %o) unnamed_addr #0 {
start:
  %_3.i = load i64, ptr %o, align 8, !range !1, !alias.scope !2, !noundef !5
  %payload.i = getelementptr inbounds { i64, i64 }, ptr %o, i64 0, i32 1
  %0 = insertvalue { ptr, i64 } undef, ptr %payload.i, 0
  %1 = insertvalue { ptr, i64 } %0, i64 %_3.i, 1
  ret { ptr, i64 } %1
}

; Function Attrs: mustprogress nofree nosync nounwind willreturn uwtable
define { ptr, i64 } @nonzero_u64_opt_as_slice(ptr noalias noundef readonly align 8 dereferenceable(8) %o) unnamed_addr #0 {
start:
  %0 = load i64, ptr %o, align 8, !alias.scope !6, !noundef !5
  %.not3.i = icmp ne i64 %0, 0
  %len.i = zext i1 %.not3.i to i64
  %1 = insertvalue { ptr, i64 } undef, ptr %o, 0
  %2 = insertvalue { ptr, i64 } %1, i64 %len.i, 1
  ret { ptr, i64 } %2
}

!1 = !{i64 0, i64 2}

[scottmcm]

Let's flip this to a hopefully-less-busy non-libs-api reviewer:
r? @the8472

[JakobDegen]

Yeah, same point as 8472 above, but I don't really see why this has to hold in the face of very silly layouts for Option<MaybeUninit<T>>. The other two I agree with.

[scottmcm]

Yup, agreed with both of you. Enough spurious padding and it could be out of bounds.

@rustbot author

[scottmcm]

As I was doing this, I realized that the layout-optimized case is actually a subset of the "guess is totally wrong case", so I think this came out pretty elegantly.

@rustbot ready

[the8472]

Maybe this could use a FIXME that what we really want here is a offset_of into an enum variant, if such a feature ever materializes. But we don't even have an RFC open that we could point to.

r=me either way

[the8472]

Wouldn't this be "inside or one past the end"?

[scottmcm]

Only if T is a ZST, since I'm checking the offset against sizeof(Option<T>) - sizeof(T), not just against sizeof(Option<T>).

("Inside" is always a hard question when it comes to ZSTs...)

[scottmcm]

@bors r=the8472

[bors]

📌 Commit e975057 has been approved by the8472

It is now in the queue for this repository.

[bors]

⌛ Testing commit e975057 with merge cf8d98b...

[bors]

☀️ Test successful - checks-actions
Approved by: the8472
Pushing cf8d98b to master...

[bors]

☀️ Test successful - checks-actions
Approved by: the8472
Pushing cf8d98b to master...

[rust-timer]

Finished benchmarking commit (cf8d98b): comparison URL.

Overall result: ❌ regressions - 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.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.8%, 0.9%]	3
Regressions ❌ (secondary)	0.6%	[0.4%, 0.7%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.8%, 0.9%]	3

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)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.9%	[-2.9%, -2.9%]	1
All ❌✅ (primary)	1.4%	[1.4%, 1.4%]	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.

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