From e0a820e4d6557a8ca87707a6c96c189826871eab Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 2 Sep 2022 12:27:18 -0400 Subject: [PATCH] avoid inferring when compilation signature differs from call site sig (#46581) Added in #43415, this was too aggressive for many cases. Unlike the comment suggested, it is unneeded in many cases, so only do it when it is expected to be maximally profitable. Fixes #46492 ``` julia> @time norm(C_212) before 45.959497 seconds (81.85 M allocations: 6.976 GiB, 6.31% gc time, 100.00% compilation time) after 15.781804 seconds (20.81 M allocations: 1.294 GiB, 6.32% gc time, 100.00% compilation time) ``` --- base/compiler/abstractinterpretation.jl | 38 ++++++++++++++----------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 98bcfa4f18661..42a1badcc8e47 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -111,7 +111,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), result = abstract_call_method(interp, method, sig_n, svec(), multiple_matches, sv) rt = result.rt edge = result.edge - edge !== nothing && push!(edges, edge) + edge === nothing || push!(edges, edge) this_argtypes = isa(matches, MethodMatches) ? argtypes : matches.applicable_argtypes[i] this_arginfo = ArgInfo(fargs, this_argtypes) const_call_result = abstract_call_method_with_const_args(interp, result, @@ -136,25 +136,11 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), this_conditional = ignorelimited(this_rt) this_rt = widenwrappedconditional(this_rt) else - if infer_compilation_signature(interp) - # Also infer the compilation signature for this method, so it's available - # to the compiler in case it ends up needing it (which is likely). - csig = get_compileable_sig(method, sig, match.sparams) - if csig !== nothing && csig !== sig - # The result of this inference is not directly used, so temporarily empty - # the use set for the current SSA value. - saved_uses = sv.ssavalue_uses[sv.currpc] - sv.ssavalue_uses[sv.currpc] = empty_bitset - abstract_call_method(interp, method, csig, match.sparams, multiple_matches, sv) - sv.ssavalue_uses[sv.currpc] = saved_uses - end - end - result = abstract_call_method(interp, method, sig, match.sparams, multiple_matches, sv) this_conditional = ignorelimited(result.rt) this_rt = widenwrappedconditional(result.rt) edge = result.edge - edge !== nothing && push!(edges, edge) + edge === nothing || push!(edges, edge) # try constant propagation with argtypes for this match # this is in preparation for inlining, or improving the return result this_argtypes = isa(matches, MethodMatches) ? argtypes : matches.applicable_argtypes[i] @@ -214,6 +200,26 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), rettype = from_interprocedural!(rettype, sv, arginfo, conditionals) + # Also considering inferring the compilation signature for this method, so + # it is available to the compiler in case it ends up needing it. + if infer_compilation_signature(interp) && 1 == seen == napplicable && rettype !== Any && rettype !== Union{} && !is_removable_if_unused(all_effects) + match = applicable[1]::MethodMatch + method = match.method + sig = match.spec_types + mi = specialize_method(match; preexisting=true) + if mi !== nothing && !const_prop_methodinstance_heuristic(interp, match, mi::MethodInstance, arginfo, sv) + csig = get_compileable_sig(method, sig, match.sparams) + if csig !== nothing && csig !== sig + # The result of this inference is not directly used, so temporarily empty + # the use set for the current SSA value. + saved_uses = sv.ssavalue_uses[sv.currpc] + sv.ssavalue_uses[sv.currpc] = empty_bitset + abstract_call_method(interp, method, csig, match.sparams, multiple_matches, sv) + sv.ssavalue_uses[sv.currpc] = saved_uses + end + end + end + if call_result_unused(sv) && !(rettype === Bottom) add_remark!(interp, sv, "Call result type was widened because the return value is unused") # We're mainly only here because the optimizer might want this code,