Skip to content

Compute effects for indirect calls in GlobalEffects#8609

Open
stevenfontanella wants to merge 2 commits intomainfrom
indirect-effects-scc
Open

Compute effects for indirect calls in GlobalEffects#8609
stevenfontanella wants to merge 2 commits intomainfrom
indirect-effects-scc

Conversation

@stevenfontanella
Copy link
Copy Markdown
Member

@stevenfontanella stevenfontanella commented Apr 15, 2026

When running in --closed-world, compute effects for indirect calls by unioning the effects of all potential functions of that type. In --closed-world, we assume that all references originate in our module, so the only possible functions that we don't know about are imports. Previously we gave up on effects analysis for indirect calls.

Yields a very small byte count reduction in calcworker (3799354 - 3799297 = 57 bytes). Also shows no significant difference in runtime: (0.1346069 -> 0.13375045 = <1% improvement, probably within noise). We expect more benefits after we're able to share indirect call effects with other passes, since currently they're only seen one layer up for callers of functions that indirectly call functions (see the newly-added tests for examples).

Followups:

  • Share effect information per type with other passes besides just via Function::effects
  • Exclude functions that don't have an address (i.e. functions that aren't the target of ref.func) from effect analysis
  • Compute effects more precisely for exact + nullable/non-nullable references

Part of #8615.

@stevenfontanella stevenfontanella changed the base branch from main to effects-scc April 16, 2026 16:28
@stevenfontanella stevenfontanella force-pushed the indirect-effects-scc branch 8 times, most recently from b7ce0b6 to 9fcf76b Compare April 16, 2026 20:59
@stevenfontanella stevenfontanella changed the title Indirect effects scc Compute effects for indirect calls in GlobalEffects Apr 16, 2026
@stevenfontanella stevenfontanella marked this pull request as ready for review April 16, 2026 21:43
@stevenfontanella stevenfontanella requested a review from a team as a code owner April 16, 2026 21:43
@stevenfontanella stevenfontanella requested review from kripken and removed request for a team April 16, 2026 21:43
Base automatically changed from effects-scc to main April 16, 2026 22:36
Comment thread src/passes/GlobalEffects.cpp Outdated
Comment thread src/passes/GlobalEffects.cpp
Comment thread src/passes/GlobalEffects.cpp Outdated
Comment thread src/passes/GlobalEffects.cpp Outdated
auto& callees = callGraph[func];
for (Name callee : info.calledFunctions) {
callees.insert(module.getFunction(callee));
std::unordered_set<HeapType> allFunctionTypes;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::unordered_set<HeapType> allFunctionTypes;
// In closed world [..]
std::unordered_set<HeapType> allFunctionTypes;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I got the comment, you wanted the below comment about open world, or did you want a comment explaining why we're noting allFunctionTypes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just suggesting that, after the previous comment that documents what we do in open world, we could document briefly how closed world is different.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Though if we merge the loops as I suggested elsewhere, maybe neither is needed?)

Comment thread src/passes/GlobalEffects.cpp
Comment thread src/passes/GlobalEffects.cpp
callGraph[caller->type.getHeapType()].insert(caller);
}

SubTypes subtypes(module);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have all the function types, we can just walk up their supertype chains to construct the edges from supertypes to subtypes rather than using a SubTypes object. This is better because constructing a SubTypes is very expensive; it traverses the entire module to find all the types.

std::vector<std::optional<EffectAnalyzer>> componentEffects;
// Points to an index in componentEffects
std::unordered_map<Function*, Index> funcComponents;
std::unordered_map<CallGraphNode, Index> funcComponents;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::unordered_map<CallGraphNode, Index> funcComponents;
std::unordered_map<CallGraphNode, Index> nodeComponents;

Comment on lines +226 to +229
// We only care about Functions that are roots, not types
// A type would be a root if a function exists with that type, but no-one
// indirect calls the type.
std::vector<CallGraphNode> allFuncs;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, but it's more obviously correct to just pass all the graph nodes into the SCC utility. And then because we have C++20 now, we can avoid creating this vector entirely and use std::views::keys(callGraph) instead!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants