[PATCH] D115847: [mlgo][inline] Improve global state tracking

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/include/llvm/Analysis/MLInlineAdvisor.h:12
 
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Analysis/InlineAdvisor.h"
----------------
not used?


================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:147
+  // The cgscc pass manager rules are such that:
+  // - if a pass leads to merging SCCs, then the pipeline is resumed
+  // - if a pass leads to splitting the SCC, then we continue with one of the
----------------
then the pipeline is restarted on the merged SCC


================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:153
+  // - in addition, if new Nodes were created by a pass (e.g. CoroSplit),
+  // they'd be adjacent to Nodes in the last SCC. So we just need to check the
+  // boundary of Nodes in NodesInLastSCC for Nodes we haven't seen. We don't
----------------
or adjacent to other new nodes. no existing passes do this, but it's possible
this isn't properly handled below, but maybe a TODO is good enough for now?


================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:158
+  for (const auto *N : NodesInLastSCC) {
+    if (N->isDead() || N->getFunction().isDeclaration())
+      continue;
----------------
does this come up? the CGSCC infra only visits function definitions


================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:164
+      const auto *AdjNode = &E.getNode();
+      if (AdjNode->isDead() || AdjNode->getFunction().isDeclaration())
+        continue;
----------------
ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115847