[PATCH] D87623: [CGSCC][NewPM] Fix adding mutually recursive new functions

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 15 18:44:20 PDT 2020


jdoerfert added a comment.

In D87623#2275540 <https://reviews.llvm.org/D87623#2275540>, @asbirlea wrote:

> I'd like to revisit this and open a discussion.

Sure. I assume we don't need to revert the patch but we can start a discussion with no pressure, right?

> The `createNode` is only called in `addNewFunctionIntoSCC` and `addNewFunctionIntoRefSCC`, where the former is only called in unit tests and the latter is only used by co-routines. I'm trying to understand what the expected changes in the CallGraph are for some of these methods, if this is specific to coroutines or there are other usecases and if we can get better testing.
> Is `registerOutlinedFunction` used out-of tree?

Except the Attributor and coroutines there is no user of the `CallGraphUpdater` (I know of).
The Attributor does not yet use this function but it is not unreasonable to expect it will be used.
There is a good chance the "shallow wrapper" functionality should actually do so, though that feature is on its own not the most useful anyway.

> @jdoerfert, @modocache: Could you help me with some context please? (I'm happy to use llvm-dev@ for the discussion too.)

I'm fine with LLVM-dev, TBH I need to talk about updating the LazyCallGraph soon anyway. I think the current `&updateCGAndAnalysisManagerForCGSCCPass` in conjunction with the `::run` method don't allow you to delete more than one function from the SCC at a time, which is bad. I might simply use it wrong but whatever it is, I'm all for a discussion on how we expose new PM CGSCC updates :)  [= how we properly replace/implement something like the `CallGraphUpdater`]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87623



More information about the llvm-commits mailing list