Add unstable loop unrolling hint attributes#156816
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
9c8f21c to
22381f6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4d232fd to
9c8493b
Compare
|
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 |
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
(would like to take a look at this as well, should have time in the next few days) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d8e3738 to
c8a1c13
Compare
|
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. |
c8a1c13 to
8ff48f5
Compare
|
The regressions in #156816 (comment) are primarily if not entirely due to the increase in the size of I could improve the size by stuffing the attributes into |
There was a problem hiding this comment.
LGTM, assuming we want to take the perf hit (which sounds reasonable). I think @JonathanBrouwer wanted to have another look?
|
I'd like to take a look, will do so in the next few days :) |
|
Then for the administration, let's r? JonathanBrouwer |
There was a problem hiding this comment.
Yeah from taking a further look I think any way to gain back the perf would seriously regress code maintainablity, so
@bors r=JonathanBrouwer,folkertdev rollup=never
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 9d862dd (parent) -> b998449 (this PR) Test differencesShow 46 test diffsStage 1
Stage 2
Additionally, 32 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard b998449636a48e2c4a362809085b600a0174e1f2 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (b998449): comparison URL. Overall result: ❌ regressions - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.0%, secondary 1.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.3%, secondary 3.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.2%, secondary 0.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 519.36s -> 524.623s (1.01%) |
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.