[PATCH] D88714: [NewPM][CGSCC] Handle newly added functions not directly referenced by existing nodes

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:516
 
-  for (Node *NewNode : NewNodes)
+  while (!NewNodes.empty()) {
+    Node *NewNode = NewNodes.pop_back_val();
----------------
The whole loop deserves a comment, along the line of: the newly created nodes are referenced by some SCC nodes via call/ref edges. We conservatively assume the newly created nodes are part of the SCC (as if they call back to the SCC).


================
Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:518
+    Node *NewNode = NewNodes.pop_back_val();
+    if (NewNode->isPopulated())
+      continue;
----------------
`assert(!NewNode->isPopulated())`


================
Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:522
 
+    SmallVector<Constant *, 16> NewNodesWorklist;
+    SmallPtrSet<Constant *, 16> NewNodesVisited;
----------------
Having the container in the loop can lead to duplicate traversal. Move NewNodesWorklist & NewNodesVisited outside the loop (and decrease 16 -> 4 may be sufficient)

Additionally, should `NewNodesVisited` affect the G.getLibFunctions() loop below which uses `Visited`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88714