assert_unsafe_precondition!: assume() the expression

[a1phyr]

This may help the compiler to get some facts about the code and thus optimize it better.

It may however have an effect on compile times

[rustbot]

r? @m-ou-se

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

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[m-ou-se]

@bors try @rust-timer queue

[bors]

⌛ Trying commit ac83eee with merge e67894d5ec350dea8f428c3445622a197639cdd1...

[saethlin]

FWIW I tried this already: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/.E2.9C.94.20codegen.20regression.20from.20intrinsics.3A.3Aunreachable.2Fllvm.2Eas.2E.2E.2E

But that was with a previous LLVM and a different implementation of const_eval_select. And I never got to the point of doing a perf run because it broke so many codegen tests. It is very interesting that this PR doesn't seem to have broken any codegen tests, I would have expected it to.

Not to open another can of worms, but if this PR regresses compile time, consider adding this change: #105435

[bors]

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

[rust-timer]

Finished benchmarking commit (e67894d5ec350dea8f428c3445622a197639cdd1): comparison URL.

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

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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)	3.1%	[0.3%, 20.4%]	232
Regressions ❌ (secondary)	3.6%	[0.3%, 24.4%]	193
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	1
Improvements ✅ (secondary)	-0.7%	[-0.8%, -0.7%]	4
All ❌✅ (primary)	3.1%	[-0.2%, 20.4%]	233

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.7%	[1.0%, 7.3%]	17
Regressions ❌ (secondary)	1.8%	[1.8%, 1.9%]	2
Improvements ✅ (primary)	-2.2%	[-14.9%, -0.4%]	96
Improvements ✅ (secondary)	-2.3%	[-5.1%, -0.7%]	130
All ❌✅ (primary)	-1.3%	[-14.9%, 7.3%]	113

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)	4.9%	[1.1%, 19.8%]	119
Regressions ❌ (secondary)	7.8%	[1.4%, 24.5%]	65
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[1.1%, 19.8%]	119

[eggyal]

Other than the suggestion I've made on the code itself, could you also update the macro's documentation to reflect this change? It currently states (in multiple places) that it only has an effect if debug_assertions are on, whereas now the compiler may assume that the precondition holds even if debug_assertions are off (and indeed it is now immediate UB if the precondition does not hold, whereas before the safety issue was due to the more subtle differences between compile-time and run-time evaluations).

[eggyal]

The macro should only be invoked from unsafe contexts, so this explicit unsafe block is superfluous (and indeed potentially harmful, as it could introduce UB if the macro is accidentally invoked from a safe context).

Suggested change

	unsafe { $crate::intrinsics::assume($e) };
	$crate::intrinsics::assume($e);

[saethlin]

Based on the perf run, I'm not sure we have any evidence this is helpful. The few improvements are in externs which might just be noise, and the rest are in CTFE and the breakdown for those is all over the place.

Can you come up with a demonstration of some code that is optimized better with this change?

[Dylan-DPC]

Closing this as there seems to be no gain or interest in this.