Page MenuHomePhabricator

[NewPM][CGSCC] Handle newly added functions in updateCGAndAnalysisManagerForPass
ClosedPublic

Authored by aeubanks on Sep 16 2020, 2:35 PM.

Details

Summary

This seems to fit the CGSCC updates model better than calling
addNewFunctionInto{Ref,}SCC() on newly created/outlined functions.
Now addNewFunctionInto{Ref,}SCC() are no longer necessary.

However, this doesn't work on newly outlined functions that aren't
referenced by the original function. e.g. if a() was outlined into b()
and c(), but c() is only referenced by b() and not by a(), this will
trigger an assert.

This also fixes an issue I was seeing with newly created functions not
having passes run on them.

Ran check-llvm with expensive checks.

Diff Detail

Event Timeline

aeubanks created this revision.Sep 16 2020, 2:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2020, 2:35 PM
aeubanks requested review of this revision.Sep 16 2020, 2:35 PM

The general comment (I mentioned this offline too), is that this is much closer to the direction I'd see the changes going. By this, I mean, the APIs removed here seemed unnecessary, and inlining their functionality at the callsite made more sense.
This seems the right approach for coroutines and outlining. I'm not clear for inlining at this point, Im working to understand the flow better there.

llvm/lib/Analysis/CGSCCPassManager.cpp
518

Remove braces.

519

I'm not clear how adding this call impacts the Inliner.

aeubanks updated this revision to Diff 293198.Sep 21 2020, 9:31 AM

rebase
remove braces

aeubanks marked an inline comment as done.Sep 21 2020, 9:35 AM
aeubanks added inline comments.
llvm/lib/Analysis/CGSCCPassManager.cpp
519

It shouldn't affect the inliner. For a given inline, the inliner will remove an edge, and potentially add new call/ref edges (corresponding to any call/ref edges in the inlined function). All of those will already have nodes since they were further down the call graph.

This would only affect completely new functions being created (or potentially new calls to functions up the call graph, which would be a bug, not sure if that's caught by asserts or not).

asbirlea accepted this revision.Sep 21 2020, 12:18 PM
asbirlea added inline comments.
llvm/lib/Analysis/CGSCCPassManager.cpp
519

Thank you for the clarification.

This revision is now accepted and ready to land.Sep 21 2020, 12:18 PM