Skip to content

Commit c05b68f

Browse files
authored
Don't inline generated functions if we can't invoke their generator (#59018)
This PR disables inlining (via an outer `src_inlining_policy` method) for method instances obtained from `@generated` methods if we cannot invoke their generator during compilation, deferring their execution to runtime via dynamic dispatch. Generated functions may be called with arguments that are not fully inferrable, yet we might inline them. The lack of concretely inferred argument types can cause the generator function to not be invoked `may_invoke_generator(mi) === false`, which causes its fallback definition to be executed when one is available to expand its body (the `else` branch of a `if @generated` construct, if present). However, fallback definitions were intended to enable more accurate type inference, but not to be actually used for execution; they will typically be slower, and in this case, it is best to defer the generation of the method body to runtime, where the generator can be invoked with concrete argument types. Implements #59013 (comment), fixing #58915.
1 parent c2fd42e commit c05b68f

File tree

5 files changed

+62
-14
lines changed

5 files changed

+62
-14
lines changed

Compiler/src/abstractinterpretation.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@ function const_prop_methodinstance_heuristic(interp::AbstractInterpreter,
12451245
if isa(code, CodeInstance)
12461246
inferred = @atomic :monotonic code.inferred
12471247
# TODO propagate a specific `CallInfo` that conveys information about this call
1248-
if src_inlining_policy(interp, inferred, NoCallInfo(), IR_FLAG_NULL)
1248+
if src_inlining_policy(interp, mi, inferred, NoCallInfo(), IR_FLAG_NULL)
12491249
return true
12501250
end
12511251
end

Compiler/src/optimize.jl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ is_declared_noinline(@nospecialize src::MaybeCompressed) =
132132
#####################
133133

134134
# return whether this src should be inlined. If so, retrieve_ir_for_inlining must return an IRCode from it
135+
136+
function src_inlining_policy(interp::AbstractInterpreter, mi::MethodInstance,
137+
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32)
138+
# If we have a generator, but we can't invoke it (because argument type information is lacking),
139+
# don't inline so we defer its invocation to runtime where we'll have precise type information.
140+
if isa(mi.def, Method) && hasgenerator(mi)
141+
may_invoke_generator(mi) || return false
142+
end
143+
return src_inlining_policy(interp, src, info, stmt_flag)
144+
end
145+
135146
function src_inlining_policy(interp::AbstractInterpreter,
136147
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32)
137148
isa(src, OptimizationState) && (src = src.src)

Compiler/src/ssair/inlining.jl

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ function resolve_todo(mi::MethodInstance, result::Union{Nothing,InferenceResult,
865865
return compileable_specialization(edge, effects, et, info, state)
866866
end
867867

868-
src_inlining_policy(state.interp, src, info, flag) ||
868+
src_inlining_policy(state.interp, mi, src, info, flag) ||
869869
return compileable_specialization(edge, effects, et, info, state)
870870

871871
add_inlining_edge!(et, edge)
@@ -897,7 +897,7 @@ function resolve_todo(mi::MethodInstance, @nospecialize(info::CallInfo), flag::U
897897
return nothing
898898
end
899899

900-
src_inlining_policy(state.interp, src, info, flag) || return nothing
900+
src_inlining_policy(state.interp, mi, src, info, flag) || return nothing
901901
ir, spec_info, debuginfo = retrieve_ir_for_inlining(cached_result, src)
902902
add_inlining_edge!(et, cached_result)
903903
return InliningTodo(mi, ir, spec_info, debuginfo, effects)
@@ -1448,7 +1448,7 @@ end
14481448
function semiconcrete_result_item(result::SemiConcreteResult,
14491449
@nospecialize(info::CallInfo), flag::UInt32, state::InliningState)
14501450
code = result.edge
1451-
mi = code.def
1451+
mi = get_ci_mi(code)
14521452
et = InliningEdgeTracker(state)
14531453

14541454
if (!OptimizationParams(state.interp).inlining || is_stmt_noinline(flag) ||
@@ -1458,7 +1458,7 @@ function semiconcrete_result_item(result::SemiConcreteResult,
14581458
(is_declared_noinline(mi.def::Method) && !is_stmt_inline(flag)))
14591459
return compileable_specialization(code, result.effects, et, info, state)
14601460
end
1461-
src_inlining_policy(state.interp, result.ir, info, flag) ||
1461+
src_inlining_policy(state.interp, mi, result.ir, info, flag) ||
14621462
return compileable_specialization(code, result.effects, et, info, state)
14631463

14641464
add_inlining_edge!(et, result.edge)
@@ -1602,10 +1602,8 @@ end
16021602

16031603
function handle_invoke_expr!(todo::Vector{Pair{Int,Any}}, ir::IRCode,
16041604
idx::Int, stmt::Expr, @nospecialize(info::CallInfo), flag::UInt32, sig::Signature, state::InliningState)
1605-
mi = stmt.args[1]
1606-
if !(mi isa MethodInstance)
1607-
mi = (mi::CodeInstance).def
1608-
end
1605+
edge = stmt.args[1]
1606+
mi = isa(edge, MethodInstance) ? edge : get_ci_mi(edge::CodeInstance)
16091607
case = resolve_todo(mi, info, flag, state)
16101608
handle_single_case!(todo, ir, idx, stmt, case, false)
16111609
return nothing
@@ -1625,13 +1623,13 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
16251623
todo = Pair{Int, Any}[]
16261624

16271625
for idx in 1:length(ir.stmts)
1628-
flag = ir.stmts[idx][:flag]
1626+
inst = ir.stmts[idx]
1627+
flag = inst[:flag]
16291628

16301629
simpleres = process_simple!(todo, ir, idx, flag, state)
16311630
simpleres === nothing && continue
16321631
stmt, sig = simpleres
1633-
1634-
info = ir.stmts[idx][:info]
1632+
info = inst[:info]
16351633

16361634
# `NativeInterpreter` won't need this, but provide a support for `:invoke` exprs here
16371635
# for external `AbstractInterpreter`s that may run the inlining pass multiple times

Compiler/src/ssair/passes.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,7 +1532,7 @@ end
15321532
function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int,
15331533
code::CodeInstance, @nospecialize(info::CallInfo), inlining::InliningState,
15341534
attach_after::Bool)
1535-
mi = code.def
1535+
mi = get_ci_mi(code)
15361536
et = InliningEdgeTracker(inlining)
15371537
if code isa CodeInstance
15381538
if use_const_api(code)
@@ -1545,7 +1545,7 @@ function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int,
15451545
return false
15461546
end
15471547

1548-
src_inlining_policy(inlining.interp, src, info, IR_FLAG_NULL) || return false
1548+
src_inlining_policy(inlining.interp, mi, src, info, IR_FLAG_NULL) || return false
15491549
src, spec_info, di = retrieve_ir_for_inlining(code, src)
15501550

15511551
# For now: Require finalizer to only have one basic block

Compiler/test/inline.jl

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2314,4 +2314,43 @@ let src = code_typed1(g_noinline_invoke, (Union{Symbol,Nothing},))
23142314
@test !any(@nospecialize(x)->isa(x,GlobalRef), src.code)
23152315
end
23162316

2317+
path = Ref{Symbol}(:unknown)
2318+
function f59018_generator(x)
2319+
if @generated
2320+
if x isa DataType && x.name === Type.body.name
2321+
path[] = :generator
2322+
return Core.sizeof(x.parameters[1])
2323+
end
2324+
else
2325+
path[] = :fallback
2326+
return Core.sizeof(x.parameters[1])
2327+
end
2328+
end
2329+
f59018() = f59018_generator(Base.inferencebarrier(Int64))
2330+
let src = code_typed1(f59018, ())
2331+
# We should hit a dynamic dispatch, because not enough information
2332+
# is available to expand the generator during compilation.
2333+
@test iscall((src, f59018_generator), src.code[end - 1])
2334+
@test path[] === :unknown
2335+
@test f59018() === 8
2336+
@test path[] === :generator
2337+
end
2338+
2339+
# https://github.com/JuliaLang/julia/issues/58915
2340+
f58915(nt) = @inline Base.setindex(nt, 2, :next)
2341+
# This function should fully-inline, i.e. it should have only built-in / intrinsic calls
2342+
# and no invokes or dynamic calls of user code
2343+
let src = code_typed1(f58915, Tuple{@NamedTuple{next::UInt32,prev::UInt32}})
2344+
# Any calls should be built-in calls
2345+
@test count(iscall(f->!isa(singleton_type(argextype(f, src)), Core.Builtin)), src.code) == 0
2346+
# There should be no invoke at all
2347+
@test count(isinvoke(Returns(true)), src.code) == 0
2348+
end
2349+
2350+
# https://github.com/JuliaLang/julia/issues/58915#issuecomment-3061421895
2351+
let src = code_typed1(Base.setindex, (@NamedTuple{next::UInt32,prev::UInt32}, Int, Symbol))
2352+
@test count(isinvoke(:merge_fallback), src.code) == 0
2353+
@test count(iscall((src, Base.merge_fallback)), src.code) == 0
2354+
end
2355+
23172356
end # module inline_tests

0 commit comments

Comments
 (0)