Use global effects call graph logic for --print-call-graph

[stevenfontanella]

Re-use the existing call graph logic from GlobalEffects to implement the --print-call-graph pass. This allows us to see indirect call edges when --closed-world is enabled. Namely we add the following edges

Caller	Callee	Meaning
Function	Function	(Already present) The caller directly calls the callee.
Function	HeapType	The caller indirectly calls the callee HeapType.
HeapType	Function	The callee has the caller HeapType. i.e. an indirect call to the HeapType may end up 'calling' the callee function.
HeapType	HeapType	The callee HeapType is a subtype of the caller HeapType. i.e. an indirect call to the supertype may end up 'calling' any of its subtypes.

Since the logic isn't necessarily general to all passes, I keep the logic in GlobalEffects rather than extracting it out.

See the rendered call graphs from the lit test:

First test

• Open world
• Closed world

Second test

• Open world
• Closed world

---

Note that the generated graph is different than what the original pass produced (even without --closed-world). See the old graph vs the new graph. Some of the style isn't preserved but isn't relevant since larger graphs would typically be imported into a separate graph visualization tool anyway.

Part of #8615.

[kripken]

How does this compare to

https://github.com/WebAssembly/binaryen/blob/main/src/passes/PrintCallGraph.cpp

?

[stevenfontanella]

I didn't know we had that! It looks like that pass only creates edges for direct calls (there's some mention of indirect calls but it looks incomplete?). In this pass we include these three types of edges that aren't there in the existing pass, as well as nodes for function HeapTypes.

I find this pass useful, maybe I can merge that into the existing pass with an option to print different types of edges?

[kripken]

sgtm to improve the existing pass. Maybe even without a new flag, just add to it?

[stevenfontanella]

sgtm to improve the existing pass. Maybe even without a new flag, just add to it?

@kripken Double-checking: we're ok with changing the behavior of the existing pass? In --closed-world, we'll add additional edges for indirect calls (described in the PR description), and even in open world, some of the style is different (also see the side-by-side comparison at the end of the PR description).

[kripken]

Wait why is PrintCallGraph now inside GlobalEffects..?

[stevenfontanella]

I mention this in the PR description, I chose to keep it here since the logic is somewhat specific to GlobalEffects. e.g. it's not obvious in general that a call graph would have edges from HeapType -> Function to indicate that the function has a given type. I could extract out the call graph logic into a shared header and then use that in both GlobalEffects and a call graph printing pass, but it seems a bit weird since I'm not sure if the call graph generation logic would be applicable anywhere else. I'm ok with going either way though.

[kripken]

I see. Well, my concern is that it seems very surprising to see a general Print* method inside GlobalEffects. PrintGlobalEffects might make sense, but this is a lot more general?

What would go into the shared header? It seems like the existing PrintCallGraph code can stay where it is, and just add edges from heap types to functions - is that more than a few lines of code?

[stevenfontanella]

For my purposes, I'd like to look at the exact same call graph that GlobalEffects is already generating, so I think I'd move this code from GlobalEffects (plus the filtering logic from #8644 once it's in) into a header and then re-use that between GlobalEffects and the print call graph pass. Does it make sense to extract it out or should I keep it here?

[kripken]

Hmm, the current PrintCallGraph pass already scans calls, and I don't think it needs effects - what would it use from the code that you linked to?

[stevenfontanella]

Sorry, the highlighted lines were a bit imprecise. I'd want to extract out buildCallGraph, but that requires us to track the indirect + direct calls from each function which we currently do in analyzeFuncs (we pass funcInfos to generateCallGraph). The current GlobalEffects also computes effects for each function at the same time that it notes the indirect + direct calls. If we extracted it out, I guess we'd just do a second pass over the functions to do that? Or maybe I'd introduce some way of composing ParallelFunctionAnalysis and running them together in one pass.

Basically if I extract out buildCallGraph it seems like we'd have to compute the call graph and the effects in two separate passes over the module's functions, is that ok?

[kripken]

Thanks, now I see, so it would track the indirect calls too.

Yes, extracting something to do that makes sense.

In fact, how about extending the existing CallGraphPropertyAnalysis from module-utils.h, with indirect call tracking? Then all existing users would benefit automatically from a more precise analysis (at least in closed world). And then both PrintCallGraph and GlobalEffects can use that utility.

[stevenfontanella]

Interesting, that looks reasonable. Let me see if I can add it there.

[tlively]

it's not obvious in general that a call graph would have edges from HeapType -> Function to indicate that the function has a given type.

I agree with @kripken that pulling this logic out into a general location sounds fine. Even if HeapType -> Function edges are not necessarily the most intuitive things, I think they are a sufficiently general and expressive way to represent this information that we may as well adopt them everywhere we care about call graphs.

[kripken]

Changes to the behavior of the pass seem fine.