Skip to content

Commit

Permalink
inlining: allow callsite inlining with cached results (#50048)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aviatesk committed Jun 4, 2023
1 parent ff23b37 commit f407a4c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
5 changes: 2 additions & 3 deletions base/compiler/typeinfer.jl
Expand Up @@ -369,10 +369,9 @@ end
function transform_result_for_cache(interp::AbstractInterpreter,
linfo::MethodInstance, valid_worlds::WorldRange, result::InferenceResult)
inferred_result = result.src
# If we decided not to optimize, drop the OptimizationState now.
# External interpreters can override as necessary to cache additional information
if inferred_result isa OptimizationState{typeof(interp)}
inferred_result = ir_to_codeinf!(inferred_result)
# TODO respect must_be_codeinf setting here?
result.src = inferred_result = ir_to_codeinf!(inferred_result)
end
if inferred_result isa CodeInfo
inferred_result.min_world = first(valid_worlds)
Expand Down
21 changes: 21 additions & 0 deletions test/compiler/inline.jl
Expand Up @@ -705,6 +705,27 @@ mktempdir() do dir
end
end

# callsite inlining with cached frames
issue49823_events = @NamedTuple{evid::Int8, base_time::Float64}[
(evid = 1, base_time = 0.0), (evid = -1, base_time = 0.0)]
issue49823_fl1(t, events) = @inline findlast(x -> x.evid (1, 4) && x.base_time <= t, events)
issue49823_fl3(t, events) = @inline findlast(x -> any(==(x.evid), (1,4)) && x.base_time <= t, events)
issue49823_fl5(t, events) = begin
f = let t=t
x -> x.evid (1, 4) && x.base_time <= t
end
@inline findlast(f, events)
end
let src = @code_typed1 issue49823_fl1(0.0, issue49823_events)
@test count(isinvoke(:findlast), src.code) == 0 # successful inlining
end
let src = @code_typed1 issue49823_fl3(0.0, issue49823_events)
@test count(isinvoke(:findlast), src.code) == 0 # successful inlining
end
let src = @code_typed1 issue49823_fl5(0.0, issue49823_events)
@test count(isinvoke(:findlast), src.code) == 0 # successful inlining
end

# Issue #42264 - crash on certain union splits
let f(x) = (x...,)
# Test splatting with a Union of non-{Tuple, SimpleVector} types that require creating new `iterate` calls
Expand Down

4 comments on commit f407a4c

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

Please sign in to comment.