Don't generate a branch hint for assertions in unoptimized builds

[saethlin]

I've been staring a lot at our unoptimized LLVM IR to reduce the perf impact of #104862 and I noticed this. Seems like an easy (if very small) win.

[rustbot]

r? @compiler-errors

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

[saethlin]

@bors try @rust-timer queue

[saethlin]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 4dda050 with merge 61161ce...

[bors]

☀️ Try build successful - checks-actions
Build commit: 61161ce (61161cecb6d684364ffdf50512793411f1c0518b)

[rust-timer]

Finished benchmarking commit (61161ce): comparison URL.

Overall result: no relevant changes - no 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.

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

Instruction count

This benchmark run did not return any relevant results for this metric.

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

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

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)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	25
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-0.0%, -0.0%]	25

Bootstrap: 635.323s -> 635.156s (-0.03%)
Artifact size: 304.45 MiB -> 304.44 MiB (-0.00%)

[compiler-errors]

Wait, can you share more about the purpose of this PR? Doesn't seem like this has any effect.

@rustbot author

[saethlin]

This doesn't have any measurable effect currently. #104862 contains this though, because it was an opportunity I noticed to generate less LLVM IR. I've significantly improved the compile time overhead of that PR (from about 40% to 4% primarily by just reducing the amount of IR that the checks generate, not by generating less checks). I am expecting to eventually have a -Cextra-ub-checks setting something like thorough or aggressive or extreme which will insert a niche check for every move of every reference. Which means in LLVM IR,

  %1 = icmp ne ptr %x, null
  %2 = call i1 @llvm.expect.i1(i1 %1, i1 true)
  br i1 %2, label %bb1, label %bb2

So for code like that, I expect that omitting a third of the instructions will have an impact. We just don't generate much code like that yet.

And I personally prefer to not land separable optimizations in a PR that is likely to have overhead. The niche checks have overhead, and this PR reduces the overhead of Assert terminators. I'd rather not end up with a perf-neutral PR and people wondering "Where's the compile time overhead of adding all this code? Is it hidden by the improvement of this optimization you did?".

[compiler-errors]

And I personally prefer to not land separable optimizations in a PR that is likely to have overhead. The niche checks have overhead, and this PR reduces the overhead of Assert terminators. I'd rather not end up with a perf-neutral PR and people wondering "Where's the compile time overhead of adding all this code? Is it hidden by the improvement of this optimization you did?".

I'm not certain if I understand this argument. On the flip side, I could say something like I don't understand why we're landing perf-neutral "optimizations" because they affect PRs we plan to land later on. I actually think it makes sense to co-locate regressions with the optimizations that most dramatically affect them.

[saethlin]

I think you understand the argument and just disagree with it :p

I'll just keep this change in the originating PR.