Skip to content

[Optimizer] Use OptimizationResult rather than OptimizationState in more places #58116

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

serenity4
Copy link
Contributor

@serenity4 serenity4 commented Apr 14, 2025

This is a data-flow refactor that removes the need to carry OptimizationState around as much, especially during finish! and in result.src for inlining. Instead, more fields have been defined for OptimizationResult to hold the minimal amount of information needed to perform these possibly required transforms:

  • compute_inlining_cost!, relying on .inlining_cost::InlineCostType and .rt::Any.
  • ir_to_codeinf!, relying on src::CodeInfo and code_info::CodeInfo. Strictly speaking, only one field is required as ir_to_codeinf!(src, ir) mutates src in place, but I'd rather avoid ambiguity.

With this in mind, transform_result_for_local_cache and transform_result_for_cache should now return an OptimizationResult, and result.src should no longer be of type OptimizationState once finish! returns.

The next step (in a future PR unless it is deemed preferable to do it in this one) would be to try to replace the volatile inference mechanism introduced in #51934 with OptimizationResult by using its ability to hold IRCode without requiring a round-trip to CodeInfo upon retrieval for inlining passes.

@serenity4
Copy link
Contributor Author

@Keno are you good with this direction in terms of data-flow? That should be in line with what we already discussed, but I had to grow OptimizationResult a bit further to avoid having weird couplings with OptimizationState and InferenceResult.

@serenity4 serenity4 marked this pull request as ready for review April 15, 2025 12:30
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.

1 participant