Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove inlining's dependence on method table lookups #39792

Merged
merged 5 commits into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
val = pure_eval_call(f, argtypes)
if val !== false
# TODO: add some sort of edge(s)
return CallMeta(val, MethodResultPure())
return CallMeta(val, MethodResultPure(info))
end
end

Expand Down Expand Up @@ -875,8 +875,10 @@ function abstract_apply(interp::AbstractInterpreter, @nospecialize(itft), @nospe
push!(retinfos, ApplyCallInfo(call.info, arginfo))
res = tmerge(res, call.rt)
if bail_out_apply(interp, res, sv)
# No point carrying forward the info, we're not gonna inline it anyway
retinfo = nothing
if i != length(ctypes)
# No point carrying forward the info, we're not gonna inline it anyway
retinfo = false
end
break
end
end
Expand Down Expand Up @@ -1074,6 +1076,26 @@ function abstract_call_unionall(argtypes::Vector{Any})
return Any
end

function abstract_invoke(interp::AbstractInterpreter, @nospecialize(ft), @nospecialize(types), @nospecialize(argtype), sv::InferenceState)
nargtype = typeintersect(types, argtype)
nargtype === Bottom && return CallMeta(Bottom, false)
nargtype isa DataType || return CallMeta(Any, false) # other cases are not implemented below
isdispatchelem(ft) || return CallMeta(Any, false) # check that we might not have a subtype of `ft` at runtime, before doing supertype lookup below
types = rewrap_unionall(Tuple{ft, unwrap_unionall(types).parameters...}, types)
nargtype = Tuple{ft, nargtype.parameters...}
Comment on lines +1084 to +1085
Copy link
Member

Choose a reason for hiding this comment

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

argtype isa DataType, but types is not, so neither is nargtype

Copy link
Member

Choose a reason for hiding this comment

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

ah, I see now we bail early in that case

Copy link
Member Author

Choose a reason for hiding this comment

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

There is an nargtype isa DataType check above. But regardless, this code just moved :)

argtype = Tuple{ft, argtype.parameters...}
result = findsup(types, method_table(interp))
if result === nothing
return CallMeta(Any, false)
end
method, valid_worlds = result
update_valid_age!(sv, valid_worlds)
(ti, env) = ccall(:jl_type_intersection_with_env, Any, (Any, Any), nargtype, method.sig)::SimpleVector
rt, edge = typeinf_edge(interp, method, ti, env, sv)
edge !== nothing && add_backedge!(edge::MethodInstance, sv)
return CallMeta(rt, InvokeCallInfo(MethodMatch(ti, env, method, argtype <: method.sig)))
end

# call where the function is known exactly
function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
fargs::Union{Nothing,Vector{Any}}, argtypes::Vector{Any},
Expand All @@ -1088,8 +1110,16 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
ft = argtype_by_index(argtypes, 3)
(itft === Bottom || ft === Bottom) && return CallMeta(Bottom, false)
return abstract_apply(interp, itft, ft, argtype_tail(argtypes, 4), sv, max_methods)
elseif f === invoke
ft = widenconst(argtype_by_index(argtypes, 2))
Copy link
Member

Choose a reason for hiding this comment

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

Should check argument count.

Copy link
Member Author

Choose a reason for hiding this comment

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

argtype_by_index will check and also handle the Vararg case correctly (similar to the apply case above).

(sigty, isexact, isconcrete, istype) = instanceof_tfunc(argtype_by_index(argtypes, 3))
(ft === Bottom || sigty === Bottom) && return CallMeta(Bottom, false)
if isexact
return abstract_invoke(interp, ft, sigty, argtypes_to_type(argtype_tail(argtypes, 4)), sv)
end
return CallMeta(Any, false)
end
return CallMeta(abstract_call_builtin(interp, f, fargs, argtypes, sv, max_methods), nothing)
return CallMeta(abstract_call_builtin(interp, f, fargs, argtypes, sv, max_methods), false)
elseif f === Core.kwfunc
if la == 2
ft = widenconst(argtypes[2])
Expand All @@ -1111,7 +1141,7 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
elseif la == 3
ub_var = argtypes[3]
end
return CallMeta(typevar_tfunc(n, lb_var, ub_var), nothing)
return CallMeta(typevar_tfunc(n, lb_var, ub_var), false)
elseif f === UnionAll
return CallMeta(abstract_call_unionall(argtypes), false)
elseif f === Tuple && la == 2 && !isconcretetype(widenconst(argtypes[2]))
Expand All @@ -1129,11 +1159,11 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
# mark !== as exactly a negated call to ===
rty = abstract_call_known(interp, (===), fargs, argtypes, sv).rt
if isa(rty, Conditional)
return CallMeta(Conditional(rty.var, rty.elsetype, rty.vtype), nothing) # swap if-else
return CallMeta(Conditional(rty.var, rty.elsetype, rty.vtype), false) # swap if-else
elseif isa(rty, Const)
return CallMeta(Const(rty.val === false), nothing)
return CallMeta(Const(rty.val === false), MethodResultPure())
Copy link
Member

Choose a reason for hiding this comment

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

definitely not pure

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not?

Copy link
Member

Choose a reason for hiding this comment

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

because rty used to have actual provenance, but just threw it away?

Copy link
Member

Choose a reason for hiding this comment

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

ah, unfolding context in github further, I see contextually we're guessing that this is the only legal answer from !==?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I'm still confused. For builtins that are in _PURE_BUILTINS and return Const we just implicitly treat the result as pure. That doesn't apply here, because !== is not a builtin, but we are treating it as if it were one that simply had its result inverted. Why does that invalidate the purity of the call?

Copy link
Member Author

Choose a reason for hiding this comment

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

jinx with your previous comment, but yes, !== is special.

end
return CallMeta(rty, nothing)
return CallMeta(rty, false)
elseif la == 3 && istopfunction(f, :(>:))
# mark issupertype as a exact alias for issubtype
# swap T1 and T2 arguments and call <:
Expand Down
9 changes: 3 additions & 6 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ struct InferenceCaches{T, S}
mi_cache::S
end

struct InliningState{S <: Union{EdgeTracker, Nothing}, T <: Union{InferenceCaches, Nothing}, V <: Union{Nothing, MethodTableView}}
struct InliningState{S <: Union{EdgeTracker, Nothing}, T <: Union{InferenceCaches, Nothing}}
params::OptimizationParams
et::S
caches::T
method_table::V
end

mutable struct OptimizationState
Expand All @@ -49,8 +48,7 @@ mutable struct OptimizationState
EdgeTracker(s_edges, frame.valid_worlds),
InferenceCaches(
get_inference_cache(interp),
WorldView(code_cache(interp), frame.world)),
method_table(interp))
WorldView(code_cache(interp), frame.world)))
return new(frame.linfo,
frame.src, frame.stmt_info, frame.mod, frame.nargs,
frame.sptypes, frame.slottypes, false,
Expand Down Expand Up @@ -85,8 +83,7 @@ mutable struct OptimizationState
nothing,
InferenceCaches(
get_inference_cache(interp),
WorldView(code_cache(interp), get_world_counter())),
method_table(interp))
WorldView(code_cache(interp), get_world_counter())))
return new(linfo,
src, stmt_info, inmodule, nargs,
sptypes_from_meth_instance(linfo), slottypes, false,
Expand Down
154 changes: 47 additions & 107 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -950,14 +950,14 @@ function inline_apply!(ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, sig::
if isa(info, UnionSplitApplyCallInfo)
if length(info.infos) != 1
# TODO: Handle union split applies?
new_info = info = nothing
new_info = info = false
Copy link
Member

Choose a reason for hiding this comment

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

Why do we prefer false over nothing here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

nothing used to mean to try looking up the method match in inference, but we don't support that any more after this. Not that we actually ever inlined these cases.

else
info = info.infos[1]
new_info = info.call
end
else
@assert info === nothing || info === false
new_info = info = nothing
new_info = info = false
end
arg_start = 3
atypes = sig.atypes
Expand Down Expand Up @@ -1016,20 +1016,23 @@ is_builtin(s::Signature) =
isa(s.f, Builtin) ||
s.ft ⊑ Builtin

function inline_invoke!(ir::IRCode, idx::Int, sig::Signature, invoke_data::InvokeData, state::InliningState, todo::Vector{Pair{Int, Any}})
function inline_invoke!(ir::IRCode, idx::Int, sig::Signature, info::InvokeCallInfo,
state::InliningState, todo::Vector{Pair{Int, Any}})
stmt = ir.stmts[idx][:inst]
calltype = ir.stmts[idx][:type]
method = invoke_data.entry
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
sig.atype, method.sig)::SimpleVector
methsp = methsp::SimpleVector
match = MethodMatch(metharg, methsp, method, true)
et = state.et
result = analyze_method!(match, sig.atypes, et, state.caches, state.params, calltype)
handle_single_case!(ir, stmt, idx, result, true, todo)
if et !== nothing
intersect!(et, WorldRange(invoke_data.min_valid, invoke_data.max_valid))

if !info.match.fully_covers
# XXX: We could union split this
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't seem like a bug (e.g. XXX)? Also not true after this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we don't have a standard notion of XXX. In this case it's just here to let people know that'd it'd be ok to do, but I didn't implement it. If can use TODO if you'd prefer. The union splitting that could be accomplished here is inlining the signature check, which isn't really a union split per-se, but the union split code kinda does as a side effect.

Copy link
Member

Choose a reason for hiding this comment

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

True, XXX just looks like flag of bad code, where this is fine

return nothing
end

atypes = sig.atypes
atype0 = atypes[2]
atypes = atypes[4:end]
pushfirst!(atypes, atype0)

result = analyze_method!(info.match, atypes, state.et, state.caches, state.params, calltype)
handle_single_case!(ir, stmt, idx, result, true, todo)
return nothing
end

Expand Down Expand Up @@ -1061,43 +1064,18 @@ function process_simple!(ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, sta
return nothing
end

# Handle invoke
invoke_data = nothing
if sig.f === Core.invoke && length(sig.atypes) >= 3
res = compute_invoke_data(sig.atypes, state.method_table)
res === nothing && return nothing
(sig, invoke_data) = res
elseif is_builtin(sig)
# No inlining for builtins (other than what was previously handled)
if sig.f !== Core.invoke && is_builtin(sig)
# No inlining for builtins (other invoke/apply)
return nothing
end

sig = with_atype(sig)

# In :invoke, make sure that the arguments we're passing are a subtype of the
# signature we're invoking.
(invoke_data === nothing || sig.atype <: invoke_data.types0) || return nothing

# Special case inliners for regular functions
if late_inline_special_case!(ir, sig, idx, stmt, state.params) || is_return_type(sig.f)
return nothing
end
return (sig, invoke_data)
end

# This is not currently called in the regular course, but may be needed
# if we ever want to re-run inlining again later in the pass pipeline after
# additional type information was discovered.
function recompute_method_matches(@nospecialize(atype), params::OptimizationParams, et::EdgeTracker, method_table::MethodTableView)
# Regular case: Retrieve matching methods from cache (or compute them)
# World age does not need to be taken into account in the cache
# because it is forwarded from type inference through `sv.params`
# in the case that the cache is nonempty, so it should be unchanged
# The max number of methods should be the same as in inference most
# of the time, and should not affect correctness otherwise.
results = findall(atype, method_table; limit=params.MAX_METHODS)
results !== missing && intersect!(et, results.valid_worlds)
MethodMatchInfo(results)
return sig
end

function analyze_single_call!(ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, @nospecialize(stmt),
Expand Down Expand Up @@ -1208,63 +1186,52 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
# todo = (inline_idx, (isva, isinvoke, na), method, spvals, inline_linetable, inline_ir, lie)
todo = Pair{Int, Any}[]
et = state.et
method_table = state.method_table
for idx in 1:length(ir.stmts)
r = process_simple!(ir, todo, idx, state)
r === nothing && continue
sig = process_simple!(ir, todo, idx, state)
sig === nothing && continue

stmt = ir.stmts[idx][:inst]
calltype = ir.stmts[idx][:type]
info = ir.stmts[idx][:info]
# Inference determined this couldn't be analyzed. Don't question it.
if info === false
continue
end

(sig, invoke_data) = r

# Check whether this call was @pure and evaluates to a constant
if calltype isa Const && info isa MethodResultPure
if is_inlineable_constant(calltype.val)
if info isa MethodResultPure
if calltype isa Const && is_inlineable_constant(calltype.val)
ir.stmts[idx][:inst] = quoted(calltype.val)
continue
end
info = info.info
end

# Inference determined this couldn't be analyzed. Don't question it.
if info === false
continue
end

# If inference arrived at this result by using constant propagation,
# it'll performed a specialized analysis for just this case. Use its
# it'll have performed a specialized analysis for just this case. Use its
# result.
if isa(info, ConstCallInfo)
handle_const_call!(ir, idx, stmt, info, sig, calltype, state.et,
state.caches, invoke_data !== nothing, todo)
handle_const_call!(ir, idx, stmt, info, sig, calltype, state.et, state.caches,
sig.f === Core.invoke, todo)
continue
end

# Ok, now figure out what method to call
if invoke_data !== nothing
inline_invoke!(ir, idx, sig, invoke_data, state, todo)
# Handle invoke
if sig.f === Core.invoke
if isa(info, InvokeCallInfo)
inline_invoke!(ir, idx, sig, info, state, todo)
end
continue
end

nu = unionsplitcost(sig.atypes)
if nu == 1 || nu > state.params.MAX_UNION_SPLITTING
if !isa(info, MethodMatchInfo)
method_table === nothing && continue
et === nothing && continue
info = recompute_method_matches(sig.atype, state.params, et, method_table)
end
# Ok, now figure out what method to call
if isa(info, MethodMatchInfo)
infos = MethodMatchInfo[info]
elseif isa(info, UnionSplitInfo)
infos = info.matches
else
if !isa(info, UnionSplitInfo)
method_table === nothing && continue
et === nothing && continue
infos = MethodMatchInfo[]
for union_sig in UnionSplitSignature(sig.atypes)
push!(infos, recompute_method_matches(argtypes_to_type(union_sig), state.params, et, method_table))
end
else
infos = info.matches
end
continue
end

analyze_single_call!(ir, todo, idx, stmt, sig, calltype, infos, state.et, state.caches, state.params)
Expand All @@ -1286,38 +1253,6 @@ function linear_inline_eligible(ir::IRCode)
return true
end

function compute_invoke_data(@nospecialize(atypes), method_table)
ft = widenconst(atypes[2])
if !isdispatchelem(ft) || has_free_typevars(ft) || (ft <: Builtin)
# TODO: this can be rather aggressive at preventing inlining of closures
# but we need to check that `ft` can't have a subtype at runtime before using the supertype lookup below
return nothing
end
invoke_tt = widenconst(atypes[3])
if !isType(invoke_tt) || has_free_typevars(invoke_tt)
return nothing
end
invoke_tt = invoke_tt.parameters[1]
if !(isa(unwrap_unionall(invoke_tt), DataType) && invoke_tt <: Tuple)
return nothing
end
if method_table === nothing
# TODO: These should be forwarded in stmt_info, just like regular
# method lookup results
return nothing
end
invoke_types = rewrap_unionall(Tuple{ft, unwrap_unionall(invoke_tt).parameters...}, invoke_tt)
invoke_entry = findsup(invoke_types, method_table)
invoke_entry === nothing && return nothing
method, valid_worlds = invoke_entry
invoke_data = InvokeData(method, invoke_types, first(valid_worlds), last(valid_worlds))
atype0 = atypes[2]
atypes = atypes[4:end]
pushfirst!(atypes, atype0)
f = singleton_type(ft)
return (Signature(f, ft, atypes), invoke_data)
end

# Check for a number of functions known to be pure
function ispuretopfunction(@nospecialize(f))
return istopfunction(f, :typejoin) ||
Expand Down Expand Up @@ -1386,6 +1321,11 @@ function late_inline_special_case!(ir::IRCode, sig::Signature, idx::Int, stmt::E
subtype_call = Expr(:call, GlobalRef(Core, :(<:)), stmt.args[3], stmt.args[2])
ir[SSAValue(idx)] = subtype_call
return true
elseif params.inlining && f === TypeVar && 2 <= length(atypes) <= 4 && (atypes[2] ⊑ Symbol)
ir[SSAValue(idx)] = Expr(:call, GlobalRef(Core, :_typevar), stmt.args[2],
length(stmt.args) < 4 ? Bottom : stmt.args[3],
length(stmt.args) == 2 ? Any : stmt.args[end])
return true
elseif is_return_type(f)
if isconstType(typ)
ir[SSAValue(idx)] = quoted(typ.parameters[1])
Expand Down
20 changes: 18 additions & 2 deletions base/compiler/stmtinfo.jl
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@ end
"""
struct MethodResultPure

This singleton represents a method result constant was proven to be
This struct represents a method result constant was proven to be
effect-free, including being no-throw (typically because the value was computed
by calling an `@pure` function).
"""
struct MethodResultPure end
struct MethodResultPure
info::Any
end
let instance = MethodResultPure(false)
global MethodResultPure
MethodResultPure() = instance
end

"""
struct UnionSplitInfo
Expand Down Expand Up @@ -94,6 +100,16 @@ struct ConstCallInfo
result::InferenceResult
end

"""
struct InvokeCallInfo

Represents a resolved call to `invoke`, carrying the Method match of the
method being processed.
"""
struct InvokeCallInfo
match::MethodMatch
end

# Stmt infos that are used by external consumers, but not by optimization.
# These are not produced by default and must be explicitly opted into by
# the AbstractInterpreter.
Expand Down
Loading