[llvm] [MLInliner] Simplify NodeCount bookkeeping (PR #96576)

Arthur Eubanks via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 13:12:10 PDT 2024


aeubanks wrote:

> > so thinking about this a bit more, it's harder to keep track of the number of live functions after #94815. you can look at `CGSCCUpdateResult::DeadFunctions` (which we don't currently pass to the InlineAdvisor, but we probably could).
> > pre-#94815, with this patch, we can't detect if another CGSCC pass will come and delete a function. currently that doesn't matter since the inliner is the only CGSCC pass that deletes functions, but I believe the current logic does allow the inline advisor to detect if another CGSCC pass deletes a function.
> > is there any reason we don't just use `Module.getFunctions().size()` for `NodeCount`?
> 
> Hmmm... good question! Can't think of a reason, and looking at the history, it seems we always did manual accounting. One possibility may be that at a point we needed to delay `delete`-ing `Function`s to the very end of inlining over the whole Module, but we're past that now.
> 
> We'd just need to remove the # of declared ones, because NodeCount is defined only - would that (the # declared) be a constant that can be determined upfront at advisor construction time?

Ah that's annoying, we can't count on # declarations being constant since you can introduce calls to intrinsics that didn't have a declaration beforehand.

I'll submit this for now since it's already an issue that MLInlineAdvisor can't handle dead functions outside of the inliner.

https://github.com/llvm/llvm-project/pull/96576


More information about the llvm-commits mailing list