[PATCH] D130717: [SCCIterator] Fix an issue in scc_member_iterator sorting

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 26 11:28:02 PDT 2023


wenlei added a comment.

> I'm tweaking the BFS walk slightly to yield a topological order.

Would be good to spell out the tweak in the description.



================
Comment at: llvm/include/llvm/ADT/SCCIterator.h:346-349
+  // Starting from nodes that have no incoming edge. These nodes are "roots" of
+  // the MST forest. This ensures that nodes are visited before their
+  // decsendents are, thus ensures hot edges are processed before cold edges,
+  // based on how MST is computed.
----------------
nit: this comment seems slightly misplaced? It describes the initialization of the queue on line 353. And slightly duplicated with the comment on line 354. Maybe move there and merge the comments. 


================
Comment at: llvm/include/llvm/ADT/SCCIterator.h:370
+      NodeInfoMap[Edge.Target].IncomingMSTEdges.erase(&Edge);
+      if (MSTEdges.count(&Edge) &&
+          NodeInfoMap[Edge.Target].IncomingMSTEdges.empty()) {
----------------
Do we need to check the visited flag still? If not, we can remove the flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130717



More information about the llvm-commits mailing list