[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