Skip to content

Commit

Permalink
remove isva field from OpaqueClosure (#44008)
Browse files Browse the repository at this point in the history
Instead allow specifying an argument tuple type separately.
  • Loading branch information
JeffBezanson authored Feb 15, 2022
1 parent 2db86f2 commit 983598a
Show file tree
Hide file tree
Showing 21 changed files with 210 additions and 176 deletions.
2 changes: 1 addition & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ eval(Core, :(CodeInstance(mi::MethodInstance, @nospecialize(rettype), @nospecial
mi, rettype, inferred_const, inferred, const_flags, min_world, max_world, ipo_effects, effects, relocatability)))
eval(Core, :(Const(@nospecialize(v)) = $(Expr(:new, :Const, :v))))
eval(Core, :(PartialStruct(@nospecialize(typ), fields::Array{Any, 1}) = $(Expr(:new, :PartialStruct, :typ, :fields))))
eval(Core, :(PartialOpaque(@nospecialize(typ), @nospecialize(env), isva::Bool, parent::MethodInstance, source::Method) = $(Expr(:new, :PartialOpaque, :typ, :env, :isva, :parent, :source))))
eval(Core, :(PartialOpaque(@nospecialize(typ), @nospecialize(env), parent::MethodInstance, source::Method) = $(Expr(:new, :PartialOpaque, :typ, :env, :parent, :source))))
eval(Core, :(InterConditional(slot::Int, @nospecialize(vtype), @nospecialize(elsetype)) = $(Expr(:new, :InterConditional, :slot, :vtype, :elsetype))))
eval(Core, :(MethodMatch(@nospecialize(spec_types), sparams::SimpleVector, method::Method, fully_covers::Bool) =
$(Expr(:new, :MethodMatch, :spec_types, :sparams, :method, :fully_covers))))
Expand Down
18 changes: 9 additions & 9 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
this_argtypes = isa(matches, MethodMatches) ? argtypes : matches.applicable_argtypes[i]
this_arginfo = ArgInfo(fargs, this_argtypes)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv, false)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv)
effects = result.edge_effects
if const_result !== nothing
(;rt, effects, const_result) = const_result
Expand Down Expand Up @@ -138,7 +138,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
# this is in preparation for inlining, or improving the return result
this_argtypes = isa(matches, MethodMatches) ? argtypes : matches.applicable_argtypes[i]
this_arginfo = ArgInfo(fargs, this_argtypes)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv, false)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv)
effects = result.edge_effects
if const_result !== nothing
this_rt = const_result.rt
Expand Down Expand Up @@ -667,7 +667,7 @@ end

function abstract_call_method_with_const_args(interp::AbstractInterpreter, result::MethodCallResult,
@nospecialize(f), arginfo::ArgInfo, match::MethodMatch,
sv::InferenceState, va_override::Bool)
sv::InferenceState)
if !const_prop_enabled(interp, sv, match)
return nothing
end
Expand Down Expand Up @@ -705,7 +705,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter, resul
return nothing
end
end
inf_result = InferenceResult(mi, (arginfo, sv), va_override)
inf_result = InferenceResult(mi, (arginfo, sv))
if !any(inf_result.overridden_by_const)
add_remark!(interp, sv, "[constprop] Could not handle constant info in matching_cache_argtypes")
return nothing
Expand Down Expand Up @@ -1457,7 +1457,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
# t, a = ti.parameters[i], argtypes′[i]
# argtypes′[i] = t ⊑ a ? t : a
# end
const_result = abstract_call_method_with_const_args(interp, result, singleton_type(ft′), arginfo, match, sv, false)
const_result = abstract_call_method_with_const_args(interp, result, singleton_type(ft′), arginfo, match, sv)
if const_result !== nothing
(;rt, const_result) = const_result
end
Expand Down Expand Up @@ -1603,7 +1603,7 @@ function abstract_call_opaque_closure(interp::AbstractInterpreter, closure::Part
const_result = nothing
if !result.edgecycle
const_result = abstract_call_method_with_const_args(interp, result, nothing,
arginfo, match, sv, closure.isva)
arginfo, match, sv)
if const_result !== nothing
(;rt, const_result) = const_result
end
Expand All @@ -1619,7 +1619,7 @@ function most_general_argtypes(closure::PartialOpaque)
if !isa(argt, DataType) || argt.name !== typename(Tuple)
argt = Tuple
end
return most_general_argtypes(closure.source, argt, closure.isva, false)
return most_general_argtypes(closure.source, argt, false)
end

# call where the function is any lattice element
Expand Down Expand Up @@ -1843,14 +1843,14 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
elseif ehead === :new_opaque_closure
tristate_merge!(sv, Effects()) # TODO
t = Union{}
if length(e.args) >= 5
if length(e.args) >= 4
ea = e.args
argtypes = collect_argtypes(interp, ea, vtypes, sv)
if argtypes === nothing
t = Bottom
else
t = _opaque_closure_tfunc(argtypes[1], argtypes[2], argtypes[3],
argtypes[4], argtypes[5], argtypes[6:end], sv.linfo)
argtypes[4], argtypes[5:end], sv.linfo)
if isa(t, PartialOpaque)
# Infer this now so that the specialization is available to
# optimization.
Expand Down
14 changes: 7 additions & 7 deletions base/compiler/inferenceresult.jl
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ end
# to return a valid value for `cache_lookup(linfo, argtypes, cache).argtypes`,
# so that we can construct cache-correct `InferenceResult`s in the first place.
function matching_cache_argtypes(
linfo::MethodInstance, (arginfo, sv)#=::Tuple{ArgInfo,InferenceState}=#, va_override::Bool)
linfo::MethodInstance, (arginfo, sv)#=::Tuple{ArgInfo,InferenceState}=#)
(; fargs, argtypes) = arginfo
@assert isa(linfo.def, Method) # ensure the next line works
nargs::Int = linfo.def.nargs
cache_argtypes, overridden_by_const = matching_cache_argtypes(linfo, nothing, va_override)
cache_argtypes, overridden_by_const = matching_cache_argtypes(linfo, nothing)
given_argtypes = Vector{Any}(undef, length(argtypes))
local condargs = nothing
for i in 1:length(argtypes)
Expand Down Expand Up @@ -55,7 +55,7 @@ function matching_cache_argtypes(
end
given_argtypes[i] = widenconditional(argtype)
end
isva = va_override || linfo.def.isva
isva = linfo.def.isva
if isva || isvarargtype(given_argtypes[end])
isva_given_argtypes = Vector{Any}(undef, nargs)
for i = 1:(nargs - isva)
Expand Down Expand Up @@ -93,8 +93,9 @@ function matching_cache_argtypes(
end

function most_general_argtypes(method::Union{Method, Nothing}, @nospecialize(specTypes),
isva::Bool, withfirst::Bool = true)
withfirst::Bool = true)
toplevel = method === nothing
isva = !toplevel && method.isva
linfo_argtypes = Any[(unwrap_unionall(specTypes)::DataType).parameters...]
nargs::Int = toplevel ? 0 : method.nargs
if !withfirst
Expand Down Expand Up @@ -192,10 +193,9 @@ function elim_free_typevars(@nospecialize t)
end
end

function matching_cache_argtypes(linfo::MethodInstance, ::Nothing, va_override::Bool)
function matching_cache_argtypes(linfo::MethodInstance, ::Nothing)
mthd = isa(linfo.def, Method) ? linfo.def::Method : nothing
cache_argtypes = most_general_argtypes(mthd, linfo.specTypes,
va_override || (isa(mthd, Method) ? mthd.isva : false))
cache_argtypes = most_general_argtypes(mthd, linfo.specTypes)
return cache_argtypes, falses(length(cache_argtypes))
end

Expand Down
11 changes: 5 additions & 6 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -235,16 +235,15 @@ function stmt_effect_free(@nospecialize(stmt), @nospecialize(rt), src::Union{IRC
elseif head === :foreigncall
return foreigncall_effect_free(stmt, src)
elseif head === :new_opaque_closure
length(args) < 5 && return false
length(args) < 4 && return false
typ = argextype(args[1], src)
typ, isexact = instanceof_tfunc(typ)
isexact || return false
typ Tuple || return false
isva = argextype(args[2], src)
rt_lb = argextype(args[3], src)
rt_ub = argextype(args[4], src)
src = argextype(args[5], src)
if !(isva Bool && rt_lb Type && rt_ub Type && src Method)
rt_lb = argextype(args[2], src)
rt_ub = argextype(args[3], src)
src = argextype(args[4], src)
if !(rt_lb Type && rt_ub Type && src Method)
return false
end
return true
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1056,10 +1056,10 @@ end

function narrow_opaque_closure!(ir::IRCode, stmt::Expr, @nospecialize(info), state::InliningState)
if isa(info, OpaqueClosureCreateInfo)
lbt = argextype(stmt.args[3], ir)
lbt = argextype(stmt.args[2], ir)
lb, exact = instanceof_tfunc(lbt)
exact || return
ubt = argextype(stmt.args[4], ir)
ubt = argextype(stmt.args[3], ir)
ub, exact = instanceof_tfunc(ubt)
exact || return
# Narrow opaque closure type
Expand All @@ -1068,7 +1068,7 @@ function narrow_opaque_closure!(ir::IRCode, stmt::Expr, @nospecialize(info), sta
# N.B.: Narrowing the ub requires a backdge on the mi whose type
# information we're using, since a change in that function may
# invalidate ub result.
stmt.args[4] = newT
stmt.args[3] = newT
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/legacy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
function inflate_ir(ci::CodeInfo, linfo::MethodInstance)
sptypes = sptypes_from_meth_instance(linfo)
if ci.inferred
argtypes, _ = matching_cache_argtypes(linfo, nothing, false)
argtypes, _ = matching_cache_argtypes(linfo, nothing)
else
argtypes = Any[ Any for i = 1:length(ci.slotflags) ]
end
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,8 @@ function lift_leaves(compact::IncrementalCompact,
ocleaf = simple_walk(compact, ocleaf)
end
ocdef, _ = walk_to_def(compact, ocleaf)
if isexpr(ocdef, :new_opaque_closure) && isa(field, Int) && 1 field length(ocdef.args)-5
lift_arg!(compact, leaf, cache_key, ocdef, 5+field, lifted_leaves)
if isexpr(ocdef, :new_opaque_closure) && isa(field, Int) && 1 field length(ocdef.args)-4
lift_arg!(compact, leaf, cache_key, ocdef, 4+field, lifted_leaves)
continue
end
return nothing
Expand Down
8 changes: 3 additions & 5 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1580,9 +1580,8 @@ function array_elmtype(@nospecialize ary)
return Any
end

function _opaque_closure_tfunc(@nospecialize(arg), @nospecialize(isva),
@nospecialize(lb), @nospecialize(ub), @nospecialize(source), env::Vector{Any},
linfo::MethodInstance)
function _opaque_closure_tfunc(@nospecialize(arg), @nospecialize(lb), @nospecialize(ub),
@nospecialize(source), env::Vector{Any}, linfo::MethodInstance)

argt, argt_exact = instanceof_tfunc(arg)
lbt, lb_exact = instanceof_tfunc(lb)
Expand All @@ -1596,9 +1595,8 @@ function _opaque_closure_tfunc(@nospecialize(arg), @nospecialize(isva),
t = lbt == ubt ? t{ubt} : (t{T} where lbt <: T <: ubt)

(isa(source, Const) && isa(source.val, Method)) || return t
(isa(isva, Const) && isa(isva.val, Bool)) || return t

return PartialOpaque(t, tuple_tfunc(env), isva.val, linfo, source.val)
return PartialOpaque(t, tuple_tfunc(env), linfo, source.val)
end

# whether getindex for the elements can potentially throw UndefRef
Expand Down
3 changes: 1 addition & 2 deletions base/compiler/typelimits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,11 @@ function tmerge(@nospecialize(typea), @nospecialize(typeb))
end
if isa(typea, PartialOpaque) && isa(typeb, PartialOpaque) && widenconst(typea) == widenconst(typeb)
if !(typea.source === typeb.source &&
typea.isva === typeb.isva &&
typea.parent === typeb.parent)
return widenconst(typea)
end
return PartialOpaque(typea.typ, tmerge(typea.env, typeb.env),
typea.isva, typea.parent, typea.source)
typea.parent, typea.source)
end
# no special type-inference lattice, join the types
typea, typeb = widenconst(typea), widenconst(typeb)
Expand Down
5 changes: 2 additions & 3 deletions base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ mutable struct InferenceResult
ipo_effects::Effects # if inference is finished
effects::Effects # if optimization is finished
function InferenceResult(linfo::MethodInstance,
arginfo#=::Union{Nothing,Tuple{ArgInfo,InferenceState}}=# = nothing,
va_override::Bool = false)
argtypes, overridden_by_const = matching_cache_argtypes(linfo, arginfo, va_override)
arginfo#=::Union{Nothing,Tuple{ArgInfo,InferenceState}}=# = nothing)
argtypes, overridden_by_const = matching_cache_argtypes(linfo, arginfo)
return new(linfo, argtypes, overridden_by_const, Any, nothing, WorldRange(), Effects(), Effects())
end
end
Expand Down
10 changes: 9 additions & 1 deletion base/opaque_closure.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

"""
@opaque (args...) -> body
@opaque ([type, ]args...) -> body
Marks a given closure as "opaque". Opaque closures capture the
world age of their creation (as opposed to their invocation).
Expand All @@ -10,9 +10,17 @@ list, but trades off against the ability to inline opaque
closures at the call site, if their creation is not statically
visible.
An argument tuple type (`type`) may optionally be specified, to
specify allowed argument types in a more flexible way. In particular,
the argument type may be fixed length even if the function is variadic.
!!! warning
This interface is experimental and subject to change or removal without notice.
"""
macro opaque(ex)
esc(Expr(:opaque_closure, ex))
end

macro opaque(ty, ex)
esc(Expr(:opaque_closure, ty, ex))
end
Loading

2 comments on commit 983598a

@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(ALL, 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.