[PATCH] D134019: [MachineCycle][NFC]add a cache from block and top level cycle mapping

Sameer Sahasrabuddhe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 16 02:24:53 PDT 2022


sameerds added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericCycleImpl.h:183
+    if (It.second == Child)
+      It.second = NewParent;
 }
----------------
nikic wrote:
> This assumes the new parent is a top level cycle. Is this guaranteed?
At the only call to this function, it seems to be true that NewParent is a newly discovered top-level cycle, and Child is an existing top-level cycle. This loop then updates the cache to replace Child with NewParent. But that makes the whole thing a bit fragile if this function is ever called from a different place. 

Maybe we should make this a private function "moveTopLevelCycleToNewParent", with suitable assertions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134019



More information about the llvm-commits mailing list