[PATCH] D81030: [JumpThreading] Simplify FindMostPopularDest (NFC)

[Kazu Hirata via Phabricator via llvm-commits]

kazu marked an inline comment as done.
kazu added a comment.

Thank you two for reviews!



================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:1490
+  for (auto *SuccBB : successors(BB))
+    DestPopularity[SuccBB] = 0;
+
----------------
efriedma wrote:
> Initializing DestPopularity like this should be unnecessary, I think; as long as the ordering of PredToDestList is deterministic, the ordering of the resulting MapVector should also be deterministic.
> 
> Can we just assert `!DestPopularity.empty()`, instead of messing with nullptr?  If PredToDestList is non-empty, DestPopularity should also be non-empty.over inserting nullptr; inserting nullptr simplifies the control flow, but it's not intuitive.
I am afraid that assuming a certain order in `PredToDestList` might make the interface a bit fragile because `FindMostPopularDest` doesn't have control over how `PredToDestList` is populated.

I agree that the `nullptr` trick is not very intuitive.  However, we cannot assert `!DestPopularity.empty()` because `DestPopularity` could be empty when the sole entry in `PredToDestList` maps  `undef` branch condition to `nullptr` destination basic block.  This situation actually occurs during `ninja check-llvm`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81030