[PATCH] D127693: [mlgo] Fix accounting for SCC splits

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

lg, some nits



================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:181
+
+  // Remember nodes in case the SCC is split before onPassExit
+  for (const auto &N : *LastSCC)
----------------
point out we're reusing NodesInLastSCC for that, so it's clear that this is the intent: 1) drain it (lines 160-180), 2) remember.

I'd even add an `assert(NodesInLastSCC.empty())` right above the for loop - same reason, clarifies the design intent

Finally: I think you can do NodesInLastSCC.insert(LastSCC.begin(), LastSCC.end()) or smth like that (skipping the for)


================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:208
     assert(!N.isDead());
+    if (NodesInLastSCC.contains(&N))
+      continue;
----------------
nit: you can do this:

```
auto I = NodesInLastSCC.insert(&N)
if (I.second)
  EdgedOfLastSeenNodes +=...
```

this way the lookup is done once


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127693