[DO NOT MERGE] Check the perf impact of reverting #139675

[Zalathar]

According to #140633 (comment), the changes in #139675 caused many measurable perf regressions. We can double-check that result by benchmarking a revert.

---

This reverts commit ed7590f, reversing changes made to 3559e0a.

r? ghost

[Zalathar]

@bors try @rust-timer queue

@rustbot experimental

[bors]

⌛ Trying commit a2c30dc with merge 4f2d7b4...

[Kobzol]

@nnethercote I seem to recall you optimizing handling of LLVM features recently. Do you see something in #139675 that could have such an effect on perf.? Granted most of it is in helloworld..

[nnethercote]

I did a Cachegrind run on the reverted code. A check full build of helloworld improved from 31.75 Minstrs to 28.56 MInstrs. The difference was all in LLVM's SetImpliedBits functions, used by feature checking, which went from 5.55 Minstrs to 2.37 MInstrs.

I don't know why the difference is so large, though. This change added two new x86 features, avx10.1-512 and avx10.2-512. This increased the number of features checked from 74 to 76, a tiny increase in relative terms. I would have expected the slow down to be much smaller than what we are seeing.

@nikic made a change to LLVM that sped up SetImpliedBits a lot: llvm/llvm-project#130936. Hopefully on the next LLVM update all this feature checking cost will plummet.

[bors]

☀️ Try build successful - checks-actions
Build commit: 4f2d7b4 (4f2d7b4b3413af1c7427caecbd9624558be0e1aa)

[rust-timer]

Finished benchmarking commit (4f2d7b4): comparison URL.

Overall result: ✅ improvements - 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 is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.3%	[-12.4%, -0.2%]	70
Improvements ✅ (secondary)	-1.9%	[-10.4%, -0.2%]	150
All ❌✅ (primary)	-1.3%	[-12.4%, -0.2%]	70

Max RSS (memory usage)

Results (primary 0.3%, secondary 0.8%)

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.3%	[0.4%, 3.6%]	8
Regressions ❌ (secondary)	0.8%	[0.8%, 0.8%]	2
Improvements ✅ (primary)	-0.7%	[-1.3%, -0.4%]	8
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-1.3%, 3.6%]	16

Cycles

Results (primary -0.4%, secondary -2.7%)

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.6%	[0.5%, 0.8%]	6
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-0.9%	[-3.9%, -0.5%]	13
Improvements ✅ (secondary)	-3.2%	[-5.4%, -2.1%]	11
All ❌✅ (primary)	-0.4%	[-3.9%, 0.8%]	19

Binary size

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

Bootstrap: 769.045s -> 770.62s (0.20%)
Artifact size: 365.44 MiB -> 365.37 MiB (-0.02%)

[nikic]

@nnethercote This is probably because avx10.1-512 and avx10.2-512 have a large number of implied features, and that used to matter prior to the referenced llvm PR.

[Zalathar]

Closing this PR, as it has served its purpose in confirming the regression.