Skip to content

Commit

Permalink
effects: change the overlayed::Bool property to nonoverlayed::Bool (
Browse files Browse the repository at this point in the history
JuliaLang#44786)

The current naming of `overlayed` is a bit confusing since
`overlayed === true`, which is the conservative default value, actually
means "it _may_ be overlayed" while `overlayed === false` means
"this is absolutely not overlayed".
I think it should be named as `nonoverlayed`, and then a query name like
`is_nonoverlayed` would be more sensible than the alternative
`is_overlayed`, which actually should be something like `can_be_overlayed`.
  • Loading branch information
aviatesk authored Mar 30, 2022
1 parent 18a2031 commit f782430
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 83 deletions.
77 changes: 39 additions & 38 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,23 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
# function has not seen any side effects, we would like to make sure there
# aren't any in the throw block either to enable other optimizations.
add_remark!(interp, sv, "Skipped call in throw block")
overlayed = true
if isoverlayed(method_table(interp))
if !sv.ipo_effects.overlayed
# as we may want to concrete-evaluate this frame in cases when there are
# no overlayed calls, try an additional effort now to check if this call
# isn't overlayed rather than just handling it conservatively
matches = find_matching_methods(arginfo.argtypes, atype, method_table(interp),
InferenceParams(interp).MAX_UNION_SPLITTING, max_methods)
if !isa(matches, FailedMethodMatch)
overlayed = matches.overlayed
end
nonoverlayed = false
if isoverlayed(method_table(interp)) && sv.ipo_effects.nonoverlayed
# as we may want to concrete-evaluate this frame in cases when there are
# no overlayed calls, try an additional effort now to check if this call
# isn't overlayed rather than just handling it conservatively
matches = find_matching_methods(arginfo.argtypes, atype, method_table(interp),
InferenceParams(interp).MAX_UNION_SPLITTING, max_methods)
if !isa(matches, FailedMethodMatch)
nonoverlayed = matches.nonoverlayed
end
else
overlayed = false
nonoverlayed = true
end
# At this point we are guaranteed to end up throwing on this path,
# which is all that's required for :consistent-cy. Of course, we don't
# know anything else about this statement.
tristate_merge!(sv, Effects(; consistent=ALWAYS_TRUE, overlayed))
tristate_merge!(sv, Effects(; consistent=ALWAYS_TRUE, nonoverlayed))
return CallMeta(Any, false)
end

Expand All @@ -88,11 +86,11 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
any_const_result = false
const_results = Union{InferenceResult,Nothing,ConstResult}[]
multiple_matches = napplicable > 1
if matches.overlayed
if !matches.nonoverlayed
# currently we don't have a good way to execute the overlayed method definition,
# so we should give up pure/concrete eval when any of the matched methods is overlayed
f = nothing
tristate_merge!(sv, Effects(EFFECTS_TOTAL; overlayed=true))
tristate_merge!(sv, Effects(EFFECTS_TOTAL; nonoverlayed=false))
end

val = pure_eval_call(interp, f, applicable, arginfo, sv)
Expand Down Expand Up @@ -212,7 +210,9 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end

if seen != napplicable
tristate_merge!(sv, Effects(; overlayed=false)) # already accounted for method overlay above
# there may be unanalyzed effects within unseen dispatch candidate,
# but we can still ignore nonoverlayed effect here since we already accounted for it
tristate_merge!(sv, EFFECTS_UNKNOWN)
elseif isa(matches, MethodMatches) ? (!matches.fullmatch || any_ambig(matches)) :
(!_all(b->b, matches.fullmatches) || any_ambig(matches))
# Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature.
Expand Down Expand Up @@ -251,7 +251,7 @@ struct MethodMatches
valid_worlds::WorldRange
mt::Core.MethodTable
fullmatch::Bool
overlayed::Bool
nonoverlayed::Bool
end
any_ambig(info::MethodMatchInfo) = info.results.ambig
any_ambig(m::MethodMatches) = any_ambig(m.info)
Expand All @@ -263,7 +263,7 @@ struct UnionSplitMethodMatches
valid_worlds::WorldRange
mts::Vector{Core.MethodTable}
fullmatches::Vector{Bool}
overlayed::Bool
nonoverlayed::Bool
end
any_ambig(m::UnionSplitMethodMatches) = _any(any_ambig, m.info.matches)

Expand All @@ -278,7 +278,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth
valid_worlds = WorldRange()
mts = Core.MethodTable[]
fullmatches = Bool[]
overlayed = false
nonoverlayed = true
for i in 1:length(split_argtypes)
arg_n = split_argtypes[i]::Vector{Any}
sig_n = argtypes_to_type(arg_n)
Expand All @@ -289,8 +289,8 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth
if result === missing
return FailedMethodMatch("For one of the union split cases, too many methods matched")
end
matches, overlayedᵢ = result
overlayed |= overlayedᵢ
matches, overlayed = result
nonoverlayed &= !overlayed
push!(infos, MethodMatchInfo(matches))
for m in matches
push!(applicable, m)
Expand All @@ -317,7 +317,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth
valid_worlds,
mts,
fullmatches,
overlayed)
nonoverlayed)
else
mt = ccall(:jl_method_table_for, Any, (Any,), atype)
if mt === nothing
Expand All @@ -337,7 +337,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth
matches.valid_worlds,
mt,
fullmatch,
overlayed)
!overlayed)
end
end

Expand Down Expand Up @@ -712,7 +712,7 @@ function concrete_eval_eligible(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState)
# disable concrete-evaluation since this function call is tainted by some overlayed
# method and currently there is no direct way to execute overlayed methods
isoverlayed(method_table(interp)) && result.edge_effects.overlayed && return false
isoverlayed(method_table(interp)) && !result.edge_effects.nonoverlayed && return false
return f !== nothing &&
result.edge !== nothing &&
is_total_or_error(result.edge_effects) &&
Expand Down Expand Up @@ -1500,13 +1500,13 @@ end
function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgInfo, sv::InferenceState)
ft′ = argtype_by_index(argtypes, 2)
ft = widenconst(ft′)
ft === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWN
ft === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWS
(types, isexact, isconcrete, istype) = instanceof_tfunc(argtype_by_index(argtypes, 3))
types === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWN
types === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWS
isexact || return CallMeta(Any, false), Effects()
argtype = argtypes_to_type(argtype_tail(argtypes, 4))
nargtype = typeintersect(types, argtype)
nargtype === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWN
nargtype === Bottom && return CallMeta(Bottom, false), EFFECTS_THROWS
nargtype isa DataType || return CallMeta(Any, false), Effects() # other cases are not implemented below
isdispatchelem(ft) || return CallMeta(Any, false), Effects() # check that we might not have a subtype of `ft` at runtime, before doing supertype lookup below
ft = ft::DataType
Expand Down Expand Up @@ -1540,6 +1540,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
(; rt, effects, const_result) = const_call_result
end
end
effects = Effects(effects; nonoverlayed=!overlayed)
return CallMeta(from_interprocedural!(rt, sv, arginfo, sig), InvokeCallInfo(match, const_result)), effects
end

Expand Down Expand Up @@ -1585,12 +1586,12 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
end
end
end
tristate_merge!(sv, Effects(; overlayed=false)) # TODO
tristate_merge!(sv, EFFECTS_UNKNOWN) # TODO
return CallMeta(Any, false)
elseif f === TypeVar
# Manually look through the definition of TypeVar to
# make sure to be able to get `PartialTypeVar`s out.
tristate_merge!(sv, Effects(; overlayed=false)) # TODO
tristate_merge!(sv, EFFECTS_UNKNOWN) # TODO
(la < 2 || la > 4) && return CallMeta(Union{}, false)
n = argtypes[2]
ub_var = Const(Any)
Expand All @@ -1603,17 +1604,17 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
end
return CallMeta(typevar_tfunc(n, lb_var, ub_var), false)
elseif f === UnionAll
tristate_merge!(sv, Effects(; overlayed=false)) # TODO
tristate_merge!(sv, EFFECTS_UNKNOWN) # TODO
return CallMeta(abstract_call_unionall(argtypes), false)
elseif f === Tuple && la == 2
tristate_merge!(sv, Effects(; overlayed=false)) # TODO
tristate_merge!(sv, EFFECTS_UNKNOWN) # TODO
aty = argtypes[2]
ty = isvarargtype(aty) ? unwrapva(aty) : widenconst(aty)
if !isconcretetype(ty)
return CallMeta(Tuple, false)
end
elseif is_return_type(f)
tristate_merge!(sv, Effects(; overlayed=false)) # TODO
tristate_merge!(sv, EFFECTS_UNKNOWN) # TODO
return return_type_tfunc(interp, argtypes, sv)
elseif la == 2 && istopfunction(f, :!)
# handle Conditional propagation through !Bool
Expand Down Expand Up @@ -1956,21 +1957,21 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
effects.effect_free ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
effects.nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
effects.terminates_globally ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
#=overlayed=#false
#=nonoverlayed=#true
))
else
tristate_merge!(sv, Effects(; overlayed=false))
tristate_merge!(sv, EFFECTS_UNKNOWN)
end
elseif ehead === :cfunction
tristate_merge!(sv, Effects(; overlayed=false))
tristate_merge!(sv, EFFECTS_UNKNOWN)
t = e.args[1]
isa(t, Type) || (t = Any)
abstract_eval_cfunction(interp, e, vtypes, sv)
elseif ehead === :method
tristate_merge!(sv, Effects(; overlayed=false))
tristate_merge!(sv, EFFECTS_UNKNOWN)
t = (length(e.args) == 1) ? Any : Nothing
elseif ehead === :copyast
tristate_merge!(sv, Effects(; overlayed=false))
tristate_merge!(sv, EFFECTS_UNKNOWN)
t = abstract_eval_value(interp, e.args[1], vtypes, sv)
if t isa Const && t.val isa Expr
# `copyast` makes copies of Exprs
Expand Down Expand Up @@ -2283,7 +2284,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
effect_free=ALWAYS_FALSE,
nothrow=TRISTATE_UNKNOWN))
elseif !isa(lhs, SSAValue)
tristate_merge!(frame, Effects(; overlayed=false))
tristate_merge!(frame, EFFECTS_UNKNOWN)
end
elseif hd === :method
stmt = stmt::Expr
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ mutable struct InferenceState
#=parent=#nothing,
#=cached=#cache === :global,
#=inferred=#false, #=dont_work_on_me=#false, #=restrict_abstract_call_sites=# isa(linfo.def, Module),
#=ipo_effects=#Effects(consistent, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false, inbounds_taints_consistency),
#=ipo_effects=#Effects(EFFECTS_TOTAL; consistent, inbounds_taints_consistency),
interp)
result.result = frame
cache !== :no && push!(get_inference_cache(interp), result)
Expand Down
10 changes: 5 additions & 5 deletions base/compiler/methodtable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ getindex(result::MethodLookupResult, idx::Int) = getindex(result.matches, idx)::
Find all methods in the given method table `view` that are applicable to the given signature `sig`.
If no applicable methods are found, an empty result is returned.
If the number of applicable methods exceeded the specified limit, `missing` is returned.
`overlayed` indicates if any matching method is defined in an overlayed method table.
`overlayed` indicates if any of the matching methods comes from an overlayed method table.
"""
function findall(@nospecialize(sig::Type), table::InternalMethodTable; limit::Int=Int(typemax(Int32)))
result = _findall(sig, nothing, table.world, limit)
Expand Down Expand Up @@ -101,23 +101,23 @@ It is possible that no method is an upper bound of `sig`, or
it is possible that among the upper bounds, there is no least element.
In both cases `nothing` is returned.
`overlayed` indicates if the matching method is defined in an overlayed method table.
`overlayed` indicates if any of the matching methods comes from an overlayed method table.
"""
function findsup(@nospecialize(sig::Type), table::InternalMethodTable)
return (_findsup(sig, nothing, table.world)..., false)
return (_findsup(sig, nothing, table.world)..., true)
end

function findsup(@nospecialize(sig::Type), table::OverlayMethodTable)
match, valid_worlds = _findsup(sig, table.mt, table.world)
match !== nothing && return match, valid_worlds, true
match !== nothing && return match, valid_worlds, false
# fall back to the internal method table
fallback_match, fallback_valid_worlds = _findsup(sig, nothing, table.world)
return (
fallback_match,
WorldRange(
max(valid_worlds.min_world, fallback_valid_worlds.min_world),
min(valid_worlds.max_world, fallback_valid_worlds.max_world)),
false)
true)
end

function _findsup(@nospecialize(sig::Type), mt::Union{Nothing,Core.MethodTable}, world::UInt)
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ function Base.show(io::IO, e::Core.Compiler.Effects)
print(io, ',')
printstyled(io, string(tristate_letter(e.terminates), 't'); color=tristate_color(e.terminates))
print(io, ')')
e.overlayed && printstyled(io, ''; color=:red)
e.nonoverlayed || printstyled(io, ''; color=:red)
end

@specialize
40 changes: 16 additions & 24 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1795,11 +1795,11 @@ function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt)
if (f === Core.getfield || f === Core.isdefined) && length(argtypes) >= 3
# consistent if the argtype is immutable
if isvarargtype(argtypes[2])
return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, overlayed=false)
return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, nonoverlayed=true)
end
s = widenconst(argtypes[2])
if isType(s) || !isa(s, DataType) || isabstracttype(s)
return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, overlayed=false)
return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, nonoverlayed=true)
end
s = s::DataType
ipo_consistent = !ismutabletype(s)
Expand Down Expand Up @@ -1828,13 +1828,10 @@ function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt)
nothrow = isvarargtype(argtypes[end]) ? false : builtin_nothrow(f, argtypes[2:end], rt)
end

return Effects(
ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
effect_free ? ALWAYS_TRUE : ALWAYS_FALSE,
nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
#=terminates=#ALWAYS_TRUE,
#=overlayed=#false,
)
return Effects(EFFECTS_TOTAL;
consistent = ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
effect_free = effect_free ? ALWAYS_TRUE : ALWAYS_FALSE,
nothrow = nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN)
end

function builtin_nothrow(@nospecialize(f), argtypes::Array{Any, 1}, @nospecialize(rt))
Expand Down Expand Up @@ -2000,24 +1997,19 @@ function intrinsic_effects(f::IntrinsicFunction, argtypes::Vector{Any})
return Effects()
end

ipo_consistent = !(f === Intrinsics.pointerref || # this one is volatile
f === Intrinsics.arraylen || # this one is volatile
ipo_consistent = !(
f === Intrinsics.pointerref || # this one is volatile
f === Intrinsics.arraylen || # this one is volatile
f === Intrinsics.sqrt_llvm_fast || # this one may differ at runtime (by a few ulps)
f === Intrinsics.have_fma || # this one depends on the runtime environment
f === Intrinsics.cglobal) # cglobal lookup answer changes at runtime

f === Intrinsics.have_fma || # this one depends on the runtime environment
f === Intrinsics.cglobal) # cglobal lookup answer changes at runtime
effect_free = !(f === Intrinsics.pointerset)
nothrow = !isvarargtype(argtypes[end]) && intrinsic_nothrow(f, argtypes[2:end])

nothrow = isvarargtype(argtypes[end]) ? false :
intrinsic_nothrow(f, argtypes[2:end])

return Effects(
ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
effect_free ? ALWAYS_TRUE : ALWAYS_FALSE,
nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
#=terminates=#ALWAYS_TRUE,
#=overlayed=#false,
)
return Effects(EFFECTS_TOTAL;
consistent = ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
effect_free = effect_free ? ALWAYS_TRUE : ALWAYS_FALSE,
nothrow = nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN)
end

# TODO: this function is a very buggy and poor model of the return_type function
Expand Down
Loading

0 comments on commit f782430

Please sign in to comment.