[PATCH] D145558: [Assignment Tracking][NFC] Use BitVectors as masks for SmallVectors rather than using DenseMaps
Scott Linder via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 20 12:10:00 PDT 2023
scott.linder accepted this revision.
scott.linder added a comment.
This revision is now accepted and ready to land.
In D145558#4199730 <https://reviews.llvm.org/D145558#4199730>, @Orlando wrote:
> Thank you @scott.linder for taking a look at this.
>
>> It might be worth including the rationale in the description/commit message, like you've done in other similar changes. I assume it is a performance improvement?
>
> Yep there's an average of around 0.25% reduction in instructions retired and a 1.1% max-rss for the CTMark suite in LTO-O3-g builds (I'll update the description).
>
>> I'm leaving a bunch of edit suggestions, but feel free to ignore them! I was just going through the exercise of writing one possible implementation to prove to myself it is an actual improvement, YMMV
>
> I appreciate the in-depth review and the suggestions all look good to me!
>
> The diff has become quite large now so I'll could look at trying to factor out some of the suggestions into NFC patches?
That is OK with me, however you prefer to land it! I am also OK with landing the previous version as-is, as I said I don't see any correctness issues
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145558/new/
https://reviews.llvm.org/D145558
More information about the llvm-commits
mailing list