[Mlir-commits] [mlir] [mlir][CallGraph] Fix abstract edge connected to external node (PR #116177)

[Markus Böck]

zero9178 wrote:

> > Conceptually makes a lot of sense to me, although the CI failure seems to be real.
> > I am also wondering whether the logic currently takes "address taken" into account. Consider e.g.:
> > ```mlir
> > func.func @foo() -> (() -> ()) {
> >    %0 = func.constant @private_func
> >    return %0
> > }
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > does this properly get a call edge from an unknown or external caller to the private function?
> > LLVM e.g. actually uses the external caller node both for public functions but also ones whose address is taken. If we don't have any edges to functions whose addresses are taken then we are now at a risk of having a too optimistic rather than the conservatice callgraph we previously had until now
> 
> I see "address taken" is considered by `struct CGUseList` in InlinerPass, `struct CGUseList` is also used for recording dead CallGraphNode. 

The inliner pass isn't necessarily the only client of `CallGraph` I'd rather see logic for the address being taken as part of the `CallGraph`. Otherwise all clients need to work around the fact that the callgraph is too optimistic the way that the inliner pass now does.  

> And the reason why CI fails is that: A dead CallGraphNode (e.g. uncalled private function) can't be visited through SCC In InlinerPass, and thus missed chance to be eliminated. I think this is not a simple error, maybe InlinerPass does too much works.

That makes a lot of sense to me, thank you! The fix implemented here (deleting dead call graph nodes) seems fine to me, although I think it'd have also been fine to adjust the test. 


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