[llvm-dev] CGSCC passes (in the new PM)

[Arthur Eubanks via llvm-dev]

I'm missing a lot of the context for the initial implementation of the NPM
CGSCC infra so I may be wrong in some aspects, but my initial thoughts are
below

On Tue, Oct 27, 2020 at 10:49 PM Johannes Doerfert <
johannesdoerfert at gmail.com> wrote:

> Hey,
>
> Recently I ran into multiple problems with the new PM CGSCC Attributor
> pass.
> Instead of going into details now, I want to first ask what we
> want/expect to work when it comes to CGSCC passes (in the new PM).
>
> What I was hoping to (eventually) do are the following things:
>
>    - Internalize functions, that is, replace all uses of
>        `define weak @foo() {}`
>      with
>        `define private @foo.internal() {}`
>      while keeping @foo around.
>
I can see issues where foo.internal() is created (say when visiting foo()),
but the CGSCC pass manager doesn't visit it, and then later on when a
caller of foo() now calls foo.internal(), the CGSCC contract is broken
since foo.internal() hasn't been visited yet.
Pretty sure that modifying all callers of foo() when splitting foo() is bad
since you'd be modifying a SCC further up.

>
>    - Delete functions, both in the current SCC but also children SCCs,
> e.g.,
>      ```
>       static void bar() { ... }
>       static void baz() { bar(); }
>       void quack() { if (/* something that is basically */ false) baz(); }
>      ```
>      We visit the singleton SCCs with bar, baz, then quack only to realize
>      when we optimize quack that baz and bar are dead and can be removed.
>
The inliner already deletes non-recursive dead functions, so I think this
should already work. (Although looking through the code, the inliner
wouldn't delete bar in your case which seems like an oversight)

>
>    - Delete multiple functions from the current SCC, this is already
>      breaking with our current (inliner-centric) update methods (AFAIKT).
>
To clarify, deleting potentially mutually recursive functions in the
current SCC? That seems reasonable, maybe by making the existing
LazyCallGraph::removeDeadFunction() more generic.

>
>    - Add/Remove/Modify globals variables. (Not related to the call graph,
>      but I wanted to mention it anyway.)
>
Not sure that modifying globals makes sense in a CGSCC pass. Maybe if the
global were only used by functions in the current SCC. Any specific
examples?

>
>    - Outline parts of a function in the current SCC into a new function
>      that will then either become part of the current SCC (even if that is
>      an overapproximation) or a child SCC.
>
I've actually been trying to work on this to make coroutines work under the
NPM. For an example see https://reviews.llvm.org/D88714. I've been meaning
to ask for more specifics on exactly what coroutines do to the call graph
to better understand what a proper solution would look like.

>
>    - Delete calls to any function.
>
> There might be more but this is all I remember right now. If it turns
> out too many things cannot be supported, it's unclear if the module
> Attributor pass is not going to be preferable, especially since we can
> parallelize the pass itself more easily (I think) than multiple
> concurrent CGSCC passes.
>
I have a feeling that concurrency at the pass manager level is a long way
off.

>
> I'd appreciate any thoughts on this :)
>
At a high level, does the attributor work on SCCs? It seems that it works
on the call graph, but not in a bottom-up way, making it different from
typical CGSCC passes. That makes me think it shouldn't be working with the
CGSCC infra. Did you want to run it alongside the inliner/other CGSCC
passes?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201028/62d82d72/attachment.html>