Sink CodeInfo transformation into transform_result_for_cache, continued
#57375
Conversation
Several downstream consumers want to cache IRCode rather than CodeInfo. Right now they need to override both `finish!` and `transform_result_for_cache`. With this change, only overriding the latter should be sufficient. As a nice bonus, we can avoid doing the work of converting to CodeInfo for interpreters that don't cache at all such as the REPL interpreter.
|
Does this supersede #56897, or in addition to it? |
|
It probably supersedes it. The idea was to test a few changes on top of it, this PR is the same but with a few more bugfixes. |
|
Test failures seem quite unrelated to me, this should be good for final review. |
|
Test failure in the |
|
The doctest is unhappy with this change. I'm fine updating the test or looking into whether there's a cfg_simplify! missing from that entry path somewhere. |
|
I had changed it before adding |
| function retrieve_ir_for_inlining(mi::MethodInstance, opt::OptimizationState, preserve_local_sources::Bool) | ||
| result = opt.result | ||
| if result !== nothing | ||
| !result.simplified && simplify_ir!(result) | ||
| return retrieve_ir_for_inlining(mi, result.ir, preserve_local_sources) | ||
| end | ||
| retrieve_ir_for_inlining(mi, opt.src, preserve_local_sources) | ||
| end |
There was a problem hiding this comment.
Is this new retrieve_ir_for_inlining (and the new OptimizationState handler of src_inlining_policy) tested somewhere? IIUC any test that would hit this.
There was a problem hiding this comment.
Never mind, because transform_result_for_cache in finish! is only applied to result that will be cached (due to isdefined(result, :ci)), inference local results and will keep the OptimizationState as is. So these new inlining code should be actually hit during the native compilation.
|
As mentioned in this comment, this PR successfully removes unnecessary conversions from |
|
Backport to 1.12? |
Continuation of the work started in #56897.
cc @Keno