[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