[PATCH] D115847: [mlgo][inline] Improve global state tracking
Mircea Trofin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 13 18:30:04 PST 2022
mtrofin added inline comments.
================
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
----------------
aeubanks wrote:
> 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?
fixed.
================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:158
+ for (const auto *N : NodesInLastSCC) {
+ if (N->isDead() || N->getFunction().isDeclaration())
+ continue;
----------------
aeubanks wrote:
> does this come up? the CGSCC infra only visits function definitions
N->isDead could happen if the function for N died since. The second part - not sure, I could imagine a pass converting an implementation to an intrinsic?
================
Comment at: llvm/lib/Analysis/MLInlineAdvisor.cpp:164
+ const auto *AdjNode = &E.getNode();
+ if (AdjNode->isDead() || AdjNode->getFunction().isDeclaration())
+ continue;
----------------
aeubanks wrote:
> ditto
here I think you're right, this should be an assert.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115847/new/
https://reviews.llvm.org/D115847
More information about the llvm-commits
mailing list