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: stop passing SemiConcreteResult to inlining_policy #52064

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Nov 7, 2023

It feels a bit inconsistent that the src argument of inlining_policy needs to handle SemiConcreteResult while it doesn't need to handle the other container objects that propagates sources like CodeInstance InferenceResult, or VolatileInferenceResult.

This commit makes inlining_policy take result.ir::IRCode instead when dealing with result::SemiConcreteResult for more consistency and clarity.

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk aviatesk force-pushed the avi/semi-concrete-inlining-policy branch from 86536a9 to aee4b87 Compare November 8, 2023 00:55
@aviatesk
Copy link
Member Author

aviatesk commented Nov 8, 2023

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

@nanosoldier
Copy link
Collaborator

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

It feels a bit inconsistent that the `src` argument of `inlining_policy`
needs to handle `SemiConcreteResult` while it doesn't need to handle
the other container objects that propagates sources like `CodeInstance`
`InferenceResult`, or `VolatileInferenceResult`.

This commit makes `inlining_policy` take `result.ir::IRCode` instead
when dealing with `result::SemiConcreteResult` for more consistency and
clarity.
@aviatesk aviatesk force-pushed the avi/semi-concrete-inlining-policy branch from aee4b87 to a77613e Compare November 9, 2023 05:42
@aviatesk
Copy link
Member Author

aviatesk commented Nov 9, 2023

Going to merge this as this cleans up the inlining algorithm fair bit.

@aviatesk aviatesk merged commit 29d78fa into master Nov 9, 2023
6 of 7 checks passed
@aviatesk aviatesk deleted the avi/semi-concrete-inlining-policy branch November 9, 2023 15:04
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