Add unstable loop unrolling hint attributes

[saethlin]

View all comments

Tracking issue: #156874

This adds as new attribute #[unroll]/#[unroll(full)]/#[unroll(never)]/#[unroll(16)] (or any u32).

#[unroll] is behind a new feature gate #![feature(loop_hints)] because I intend to add an attribute for loop vectorization as well. If a user wants to turn off loop unrolling to locally minimize code size, LLVM may vectorize the loop even though it isn't unrolled which can produce a similar code size explosion.

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred in coverage instrumentation.

cc @Zalathar

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[JonathanBrouwer]

(would like to take a look at this as well, should have time in the next few days)

[folkertdev]

Looks reasonable, I'll let Jonathan take a closer look at the attribute stuff.

Maybe I'm missing something or am just used to different parts of the code base, but some style things seemed off to me. Feel free to disregard that though, I guess.

View changes since this review

[folkertdev]

is checking the actual number tricky? or does it not map one-to-one?

[saethlin]

I am checking the number, it's at the very bottom of this file. Loop metadata looks like this:

bb7:                                              ; preds = %bb3
  call void @maybe_has_side_effect() #5
  br label %bb1, !llvm.loop !11 
...
!11 = distinct !{!11, !12}
!12 = !{!"llvm.loop.unroll.count", i32 5}

[saethlin]

I think the image perf result is just something broken in collection. https://rust-lang.zulipchat.com/#narrow/channel/247081-t-compiler.2Fperformance/topic/sus.20perf.20results/near/599190868

[Kobzol]

Let's try again to see if it persists.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 57c302e (57c302e0d23431b9eb5bf77a5403230fb921e506, parent: 968d50ad35115bc2c8c19cb9039f7ed3dfe56a81)

[rust-timer]

Finished benchmarking commit (57c302e): 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.2%	[0.1%, 0.3%]	21
Regressions ❌ (secondary)	0.2%	[0.0%, 0.3%]	21
Improvements ✅ (primary)	-36.8%	[-77.8%, -11.1%]	4
Improvements ✅ (secondary)	-0.4%	[-0.6%, -0.1%]	3
All ❌✅ (primary)	-5.7%	[-77.8%, 0.3%]	25

Max RSS (memory usage)

Results (primary -2.1%, secondary 1.6%)

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

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.6%, 4.6%]	10
Regressions ❌ (secondary)	1.6%	[0.5%, 2.8%]	12
Improvements ✅ (primary)	-14.8%	[-24.2%, -1.9%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.1%	[-24.2%, 4.6%]	13

Cycles

Results (primary -37.5%)

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)	-	-	0
Improvements ✅ (primary)	-37.5%	[-78.5%, -11.0%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-37.5%	[-78.5%, -11.0%]	4

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.7%]	87
Regressions ❌ (secondary)	0.4%	[0.0%, 1.4%]	70
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.7%]	87

Bootstrap: 511.125s -> 514.465s (0.65%)
Artifact size: 400.79 MiB -> 401.10 MiB (0.08%)

[saethlin]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: c1e5e0f (c1e5e0ffb239a664235b939ffad67af5426f4d9f, parent: 4a31759ad18b3c29c5ec99ca23c4764a8bedcf52)

[rust-timer]

Finished benchmarking commit (c1e5e0f): comparison URL.

Overall result: ❌ regressions - 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.2%	[0.1%, 0.3%]	17
Regressions ❌ (secondary)	0.2%	[0.0%, 0.6%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	0.2%	[0.1%, 0.3%]	17

Max RSS (memory usage)

Results (primary 0.1%, secondary 2.9%)

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

	mean	range	count
Regressions ❌ (primary)	1.3%	[0.5%, 2.2%]	3
Regressions ❌ (secondary)	4.7%	[1.4%, 6.6%]	3
Improvements ✅ (primary)	-3.7%	[-3.7%, -3.7%]	1
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	0.1%	[-3.7%, 2.2%]	4

Cycles

Results (primary -2.1%, secondary 5.1%)

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)	5.1%	[2.9%, 7.3%]	2
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Binary size

Results (primary -0.0%, secondary -0.0%)

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

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	12
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	11
Improvements ✅ (primary)	-0.0%	[-0.1%, -0.0%]	26
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	20
All ❌✅ (primary)	-0.0%	[-0.1%, 0.0%]	38

Bootstrap: 511.17s -> 513.02s (0.36%)
Artifact size: 400.72 MiB -> 400.98 MiB (0.06%)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.