Skip to content

Make MandatoryInlining a SILFunctionTransform #28394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

zoecarver
Copy link
Contributor

This patch is the first in a few that will clean up MandatoryInlining. For ease of reviewing, I am only changing MandatoryInlining from a SILModuleTransform to a SILFunctionTransform in this patch. Next, I'm going to remove the function removal cleanup from MandatoryInlining and move any missing functionality to MandatoryCombine.

cc @gottesmm

@gottesmm
Copy link
Contributor

A few things:

  1. Part of this is that the design will need to be changed. Mandatory Inlining currently processes functions recursively and processes Callees. Instead what we should do is rely on the pass manager to instead schedule those changes.

  2. It is important that you make sure that when mandatory inlining links in functions, the linked in function is added to the pass manager worklist.

@gottesmm
Copy link
Contributor

I would also look at how the performance inlined handles this:

https://github.com/apple/swift/blob/master/lib/SILOptimizer/Transforms/PerformanceInliner.cpp

@zoecarver
Copy link
Contributor Author

I realized that I couldn't make this a SILFunctionTransform without trying to remove every function in the module. #28396 fixes that. I think its best if I only move forward with #28396.

@gottesmm thank you for the comments; I'll address them in my other patch.

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.

2 participants