Skip to content

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
@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.

3 participants