[PATCH] D87623: [CGSCC][NewPM] Fix adding mutually recursive new functions

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 15 15:25:08 PDT 2020


aeubanks added inline comments.


================
Comment at: llvm/lib/Analysis/LazyCallGraph.cpp:1599
-  assert(!lookup(F) && "node already exists");
-
   Node &N = get(F);
----------------
jdoerfert wrote:
> If lookup succeeds, should we just return the result or does it make sense to re-populate etc? We could call this getOrCreateNode?
get() will create a node but not populate it. In the adding mutually recursive functions case, we will have called get() on a new function, creating a new node, but not populate it. Then when we add the second function, the corresponding node will exist but not be populated, so it needs to be populated then.

So I think the current name still makes sense, it's "creating" a node in the sense that it populates it, although the node may have already been allocated. Open to suggestion though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87623



More information about the llvm-commits mailing list