[PATCH] D144732: [Assignment Tracking] Elide a map copy in some cases

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 04:07:57 PDT 2023


Orlando planned changes to this revision.
Orlando marked 3 inline comments as done.
Orlando added a comment.

> Could you add the context for review

Will do, sorry about that.

> Are there any clear winners in CTMark that justify this change? 0.05% is close to noise, and the function complexity increases substantially.

Good question, -0.22% for tramp3d-v4 and -0.11% for mafft in that suite. However, it's probably worth me checking if the improvement is worth it after D145558 <https://reviews.llvm.org/D145558>.



================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:1823-1824
   // (expensive check).
   if (CurrentLiveInEntry == LiveIn.end() ||
       BBLiveIn != CurrentLiveInEntry->second) {
     LiveIn[&BB] = std::move(BBLiveIn);
----------------
StephenTozer wrote:
> I see that above for the single-visited-pred case you've split this `if` into two separate checks, so that you can use the found iterator `CurrentLiveInEntry` if it's valid. Since we're doing essentially the same thing here, would it make sense to use the same logic in both places, or is there a reason for the difference?
Originally for line count / simplicity, but at this point it doesn't really make a difference. Changed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144732/new/

https://reviews.llvm.org/D144732



More information about the llvm-commits mailing list