-
-
Notifications
You must be signed in to change notification settings - Fork 15.8k
Adressing compile time regression from loop hint attributes #161217
Copy link
Copy link
Open
Labels
A-attributesArea: Attributes (`#[…]`, `#![…]`)Area: Attributes (`#[…]`, `#![…]`)F-loop-hints`#![feature(loop_hints)]``#![feature(loop_hints)]`I-compiletimeIssue: Problems and improvements with respect to compile times.Issue: Problems and improvements with respect to compile times.needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.This issue may need triage. Remove it if it has been sufficiently triaged.
Description
Activity
Metadata
Metadata
Assignees
Labels
A-attributesArea: Attributes (`#[…]`, `#![…]`)Area: Attributes (`#[…]`, `#![…]`)F-loop-hints`#![feature(loop_hints)]``#![feature(loop_hints)]`I-compiletimeIssue: Problems and improvements with respect to compile times.Issue: Problems and improvements with respect to compile times.needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.This issue may need triage. Remove it if it has been sufficiently triaged.
#156816 added new loop hint attributes (
unrolland various friends). This caused a small compile time regression. At the time of merge, the regression was believed to be caused by addingattributesfield toTerminator, which increased Terminator (andBasicBlockData) size by 8 bytes.However, this doesn't seem to be the case (at least anymore). I tried to mess around with the Terminator in various ways to decrese its size, but I wasn't able to get any significant changes. Here's a few pieces of evidence:
Terminator::attributes.It's possible that the original analysis was correct, but something changed since then:
The bottleneck shifted somewhere else in the meantime. For example, BasicBlockData size was 160 bytes at the time of writing, but it was reduced to 144 bytes since.
The regression was fixed by something else (e.g. LLVM PR which optimized the releated rustc code better).
Either way, the original justification for accepting the regression doesn't seem to apply anymore, which means it might be worth addressing.
Some TODO items to try to narrow it down:
br_with_attrscost, maybe outline the cold attribute list processing (there might be some overhead in processing the empty list that LLVM is not able to optimize). Doesn't seem to be likely to me.That said, based on eyeballing the detailed results in the original PR, I think the cost is somewhere before MIR, so that's probably the place to investigate.
The regression is small, so it might not be worth the opportunity cost, but I already sank some time into it so I figured I might as well just write this down so it's not forgotten.