From f782430c5b7948facec0e35cd04d928f55dcea91 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Wed, 30 Mar 2022 20:12:11 +0900 Subject: [PATCH] effects: change the `overlayed::Bool` property to `nonoverlayed::Bool` (#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`. --- base/compiler/abstractinterpretation.jl | 77 +++++++++++++------------ base/compiler/inferencestate.jl | 2 +- base/compiler/methodtable.jl | 10 ++-- base/compiler/ssair/show.jl | 2 +- base/compiler/tfuncs.jl | 40 +++++-------- base/compiler/types.jl | 25 ++++---- src/julia.h | 4 +- 7 files changed, 77 insertions(+), 83 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 329988744fe23..29a9f9edd3001 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -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 @@ -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) @@ -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. @@ -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) @@ -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) @@ -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) @@ -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) @@ -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 @@ -337,7 +337,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth matches.valid_worlds, mt, fullmatch, - overlayed) + !overlayed) end end @@ -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) && @@ -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 @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl index 7d2f69cbab311..0790b18bf83bd 100644 --- a/base/compiler/inferencestate.jl +++ b/base/compiler/inferencestate.jl @@ -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) diff --git a/base/compiler/methodtable.jl b/base/compiler/methodtable.jl index da493cf9a9ef5..f2e7a4ec2cb2c 100644 --- a/base/compiler/methodtable.jl +++ b/base/compiler/methodtable.jl @@ -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) @@ -101,15 +101,15 @@ 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 ( @@ -117,7 +117,7 @@ function findsup(@nospecialize(sig::Type), table::OverlayMethodTable) 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) diff --git a/base/compiler/ssair/show.jl b/base/compiler/ssair/show.jl index 76cbcbd4d5d7d..f4c826a45156f 100644 --- a/base/compiler/ssair/show.jl +++ b/base/compiler/ssair/show.jl @@ -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 diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index 88f335cc6d492..597519dee67e1 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -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) @@ -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)) @@ -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 diff --git a/base/compiler/types.jl b/base/compiler/types.jl index 0d8275ac0e7a0..6e04e1c3ba9bb 100644 --- a/base/compiler/types.jl +++ b/base/compiler/types.jl @@ -38,7 +38,7 @@ struct Effects effect_free::TriState nothrow::TriState terminates::TriState - overlayed::Bool + nonoverlayed::Bool # This effect is currently only tracked in inference and modified # :consistent before caching. We may want to track it in the future. inbounds_taints_consistency::Bool @@ -48,33 +48,34 @@ function Effects( effect_free::TriState, nothrow::TriState, terminates::TriState, - overlayed::Bool) + nonoverlayed::Bool) return Effects( consistent, effect_free, nothrow, terminates, - overlayed, + nonoverlayed, false) end -const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false) -const EFFECTS_THROWN = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_FALSE, ALWAYS_TRUE, false) -const EFFECTS_UNKNOWN = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, true) +const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, true) +const EFFECTS_THROWS = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_FALSE, ALWAYS_TRUE, true) +const EFFECTS_UNKNOWN = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, true) # mostly unknown, but it's not overlayed at least (e.g. it's not a call) +const EFFECTS_UNKNOWN′ = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, false) # unknown, really -function Effects(e::Effects = EFFECTS_UNKNOWN; +function Effects(e::Effects = EFFECTS_UNKNOWN′; consistent::TriState = e.consistent, effect_free::TriState = e.effect_free, nothrow::TriState = e.nothrow, terminates::TriState = e.terminates, - overlayed::Bool = e.overlayed, + nonoverlayed::Bool = e.nonoverlayed, inbounds_taints_consistency::Bool = e.inbounds_taints_consistency) return Effects( consistent, effect_free, nothrow, terminates, - overlayed, + nonoverlayed, inbounds_taints_consistency) end @@ -97,7 +98,7 @@ function encode_effects(e::Effects) (e.effect_free.state << 2) | (e.nothrow.state << 4) | (e.terminates.state << 6) | - (UInt32(e.overlayed) << 8) + (UInt32(e.nonoverlayed) << 8) end function decode_effects(e::UInt32) return Effects( @@ -119,7 +120,7 @@ function tristate_merge(old::Effects, new::Effects) old.nothrow, new.nothrow), tristate_merge( old.terminates, new.terminates), - old.overlayed | new.overlayed, + old.nonoverlayed & new.nonoverlayed, old.inbounds_taints_consistency | new.inbounds_taints_consistency) end @@ -169,7 +170,7 @@ mutable struct InferenceResult 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(; overlayed=false), Effects(; overlayed=false), nothing) + WorldRange(), Effects(), Effects(), nothing) end end diff --git a/src/julia.h b/src/julia.h index d0b31c303ed6c..b4604ff86d00e 100644 --- a/src/julia.h +++ b/src/julia.h @@ -397,7 +397,7 @@ typedef struct _jl_code_instance_t { uint8_t ipo_effect_free:2; uint8_t ipo_nothrow:2; uint8_t ipo_terminates:2; - uint8_t ipo_overlayed:1; + uint8_t ipo_nonoverlayed:1; } ipo_purity_flags; }; union { @@ -407,7 +407,7 @@ typedef struct _jl_code_instance_t { uint8_t effect_free:2; uint8_t nothrow:2; uint8_t terminates:2; - uint8_t overlayed:1; + uint8_t nonoverlayed:1; } purity_flags; }; jl_value_t *argescapes; // escape information of call arguments