Preserve CallMeta results during inference#640
Merged
Conversation
Collaborator
Author
|
@aviatesk would you have some time to take a look? |
Keno
added a commit
to JuliaLang/julia
that referenced
this pull request
May 6, 2025
This makes invoke inlining work for CallInfo wrappers, e.g. - JuliaComputing/DAECompiler.jl#23 - JuliaDebug/Cthulhu.jl#640 I think this is probably the cleanest way to do it. Other ways might be to refactor InvokeCallInfo to wrap around a generic call info, but we already have this interface for callinfos, so might as well try to use it.
Keno
added a commit
to JuliaLang/julia
that referenced
this pull request
May 8, 2025
This makes invoke inlining work for CallInfo wrappers, e.g. - JuliaComputing/DAECompiler.jl#23 - JuliaDebug/Cthulhu.jl#640 I think this is probably the cleanest way to do it. Other ways might be to refactor InvokeCallInfo to wrap around a generic call info, but we already have this interface for callinfos, so might as well try to use it.
Keno
added a commit
to JuliaLang/julia
that referenced
this pull request
May 9, 2025
This makes invoke inlining work for CallInfo wrappers, e.g. - JuliaComputing/DAECompiler.jl#23 - JuliaDebug/Cthulhu.jl#640 I think this is probably the cleanest way to do it. Other ways might be to refactor InvokeCallInfo to wrap around a generic call info, but we already have this interface for callinfos, so might as well try to use it.
charleskawczynski
pushed a commit
to charleskawczynski/julia
that referenced
this pull request
May 12, 2025
This makes invoke inlining work for CallInfo wrappers, e.g. - JuliaComputing/DAECompiler.jl#23 - JuliaDebug/Cthulhu.jl#640 I think this is probably the cleanest way to do it. Other ways might be to refactor InvokeCallInfo to wrap around a generic call info, but we already have this interface for callinfos, so might as well try to use it.
aviatesk
approved these changes
May 13, 2025
Member
aviatesk
left a comment
aviatesk
left a comment
There was a problem hiding this comment.
Sorry for the significant delay in reviewing this.
I think the overall approach and implementation look good. Thanks.
The necessary special case for UnionSplitApplyCallInfo is a bit of a pain, but I think it's okay to hold off on a fundamental fix (like preparing an interface on the inlining algorithm side) until we actually run into an issue.
LGTM.
Collaborator
Author
|
No worries, I understand you've been quite busy these days :) thanks for the review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Store
CallMetainformation emitted as a result of call inference, so that we may use it during analysis once inference is done. It should theoretically provide more refinedrt/exct/effectsvalues, and it additionally containsrefinementsinformation should we ever want to support introspecting into that (which could prove helpful e.g. for JuliaLang/julia#57651).This is performed by defining
CthulhuCallInfo <: CC.CallInfowhich is used as a wrapper for (almost) allCC.CallInfostructures, applied right after inference and propagated throughout optimization (notably, through inlining). Special handling had to be done forCC.UnionSplitApplyCallInfo, but as far I as am aware this should not be limiting in any way.We might be able to refactor and trim a few of our
CallInfostructures with this change, I plan to tackle that next. In the current state, I think this can be merged safely to keep changes incremental and small, if we are willing to commit to this design.I'd be happy to have any pointers on which example may bring to light the theoretical improvements of keeping
CallMetainformation forrt/exct/effects, so we can add a test and have this PR be a clear and net improvement overall.Many thanks to @aviatesk for suggesting this approach and paving the way with explanations on how to integrate custom
CC.CallInfointo the compilation pipeline.