Skip to content
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

inlining: allow callsite inlining with cached results #50048

Merged
merged 1 commit into from
Jun 4, 2023

Conversation

aviatesk
Copy link
Sponsor Member

@aviatesk aviatesk commented Jun 3, 2023

In some rare cases with callsite inlining, we try to inline an inferred result from a local cache (inf_result::InferenceResult), whose source has been transformed by transform_result_for_cache. At present, inf_result.src stays to be OptimizationState in such cases, causing inlining_policy to handle the callsite inlining.

This commit adjusts transform_result_for_cache so that it stores the transformed source in inf_result.src, letting the callsite inliner use it. Down the line, we might revisit this change to align it with 532125d, which isn't enabled yet.

@aviatesk aviatesk requested a review from Keno June 3, 2023 08:56
@aviatesk aviatesk added the compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) label Jun 3, 2023
@aviatesk aviatesk added the backport 1.9 Change should be backported to release-1.9 label Jun 3, 2023
In some rare cases with callsite inlining, we try to inline an inferred
result from a local cache (`inf_result::InferenceResult`), whose source
has been transformed by `transform_result_for_cache`. At present,
`inf_result.src` stays to be `OptimizationState` in such cases,
causing `inlining_policy` to handle the callsite inlining.

This commit adjusts `transform_result_for_cache` so that it stores the
transformed source in `inf_result.src`, letting the callsite inliner
use it. Down the line, we might revisit this change to align it with
532125d, which isn't enabled yet.
@aviatesk
Copy link
Sponsor Member Author

aviatesk commented Jun 4, 2023

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

@aviatesk aviatesk merged commit f407a4c into master Jun 4, 2023
7 checks passed
@aviatesk aviatesk deleted the avi/callsite-inlining branch June 4, 2023 03:20
KristofferC pushed a commit that referenced this pull request Jun 6, 2023
In some rare cases with callsite inlining, we try to inline an inferred
result from a local cache (`inf_result::InferenceResult`), whose source
has been transformed by `transform_result_for_cache`. At present,
`inf_result.src` stays to be `OptimizationState` in such cases,
causing `inlining_policy` to handle the callsite inlining.

This commit adjusts `transform_result_for_cache` so that it stores the
transformed source in `inf_result.src`, letting the callsite inliner
use it. Down the line, we might revisit this change to align it with
532125d, which isn't enabled yet.

(cherry picked from commit f407a4c)
@KristofferC KristofferC mentioned this pull request Jun 6, 2023
36 tasks
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:optimizer Optimization passes (mostly in base/compiler/ssair/)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants