From 8307dbfe7c143f953853eaddd933d44c19f62e2a Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 30 Aug 2023 16:23:08 +0000 Subject: [PATCH 1/3] improve effects of typejoin and simplify implementation of Tuple _compute_eltype - Repair definition of Core._foldable_meta - Handle Vararg better in eltype - Mark has_free_typevars ccall as total - Mark tailjoin foldable explicitly also, since it calls typejoin recursively - Handle non-constructable Tuple{:a,2} and Tuple{T} types - Prepare special code for inlining (improves generic effects too) - Only optimize typejoin in concrete eval, not needlessly generating specializations otherwise on specific types The compiler seems better able to deal with this version of _compute_eltype, and it does not seem that necessary to form the IdSet. --- base/boot.jl | 36 ++++++++++---------- base/compiler/abstractinterpretation.jl | 29 ++++++++++------ base/essentials.jl | 5 ++- base/promotion.jl | 13 +++++--- base/reflection.jl | 2 +- base/tuple.jl | 44 ++++++++++--------------- 6 files changed, 67 insertions(+), 62 deletions(-) diff --git a/base/boot.jl b/base/boot.jl index e24a6f4ffc0e0..a7d7f888caf70 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -249,11 +249,25 @@ macro nospecialize(x) _expr(:meta, :nospecialize, x) end -TypeVar(n::Symbol) = _typevar(n, Union{}, Any) -TypeVar(n::Symbol, @nospecialize(ub)) = _typevar(n, Union{}, ub) -TypeVar(n::Symbol, @nospecialize(lb), @nospecialize(ub)) = _typevar(n, lb, ub) +_is_internal(__module__) = __module__ === Core +# can be used in place of `@assume_effects :foldable` (supposed to be used for bootstrapping) +macro _foldable_meta() + return _is_internal(__module__) && _expr(:meta, _expr(:purity, + #=:consistent=#true, + #=:effect_free=#true, + #=:nothrow=#false, + #=:terminates_globally=#true, + #=:terminates_locally=#false, + #=:notaskstate=#true, + #=:inaccessiblememonly=#true, + #=:noub=#true)) +end -UnionAll(v::TypeVar, @nospecialize(t)) = ccall(:jl_type_unionall, Any, (Any, Any), v, t) +# n.b. the effects and model of these is refined in inference abstractinterpretation.jl +TypeVar(@nospecialize(n)) = _typevar(n::Symbol, Union{}, Any) +TypeVar(@nospecialize(n), @nospecialize(ub)) = _typevar(n::Symbol, Union{}, ub) +TypeVar(@nospecialize(n), @nospecialize(lb), @nospecialize(ub)) = _typevar(n::Symbol, lb, ub) +UnionAll(@nospecialize(v), @nospecialize(t)) = ccall(:jl_type_unionall, Any, (Any, Any), v::TypeVar, t) # simple convert for use by constructors of types in Core # note that there is no actual conversion defined here, @@ -453,20 +467,6 @@ function _Task(@nospecialize(f), reserved_stack::Int, completion_future) return ccall(:jl_new_task, Ref{Task}, (Any, Any, Int), f, completion_future, reserved_stack) end -_is_internal(__module__) = __module__ === Core -# can be used in place of `@assume_effects :foldable` (supposed to be used for bootstrapping) -macro _foldable_meta() - return _is_internal(__module__) && Expr(:meta, Expr(:purity, - #=:consistent=#true, - #=:effect_free=#true, - #=:nothrow=#false, - #=:terminates_globally=#true, - #=:terminates_locally=#false, - #=:notaskstate=#false, - #=:inaccessiblememonly=#false, - #=:noub=#true)) -end - const NTuple{N,T} = Tuple{Vararg{T,N}} ## primitive Array constructors diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 38b33e51cda5f..92bb8da22d73a 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -847,7 +847,7 @@ function concrete_eval_eligible(interp::AbstractInterpreter, end # disable concrete-evaluation if this function call is tainted by some overlayed # method since currently there is no easy way to execute overlayed methods - add_remark!(interp, sv, "[constprop] Concrete evel disabled for overlayed methods") + add_remark!(interp, sv, "[constprop] Concrete eval disabled for overlayed methods") end if !any_conditional(arginfo) return :semi_concrete_eval @@ -1852,7 +1852,7 @@ function abstract_call_builtin(interp::AbstractInterpreter, f::Builtin, (; fargs return rt end -function abstract_call_unionall(interp::AbstractInterpreter, argtypes::Vector{Any}) +function abstract_call_unionall(interp::AbstractInterpreter, argtypes::Vector{Any}, call::CallMeta) if length(argtypes) == 3 canconst = true a2 = argtypes[2] @@ -1865,10 +1865,10 @@ function abstract_call_unionall(interp::AbstractInterpreter, argtypes::Vector{An body = a3.parameters[1] canconst = false else - return CallMeta(Any, Effects(EFFECTS_TOTAL; nothrow), NoCallInfo()) + return CallMeta(Any, Effects(EFFECTS_TOTAL; nothrow), call.info) end if !(isa(body, Type) || isa(body, TypeVar)) - return CallMeta(Any, EFFECTS_THROWS, NoCallInfo()) + return CallMeta(Any, EFFECTS_THROWS, call.info) end if has_free_typevars(body) if isa(a2, Const) @@ -1877,13 +1877,13 @@ function abstract_call_unionall(interp::AbstractInterpreter, argtypes::Vector{An tv = a2.tv canconst = false else - return CallMeta(Any, EFFECTS_THROWS, NoCallInfo()) + return CallMeta(Any, EFFECTS_THROWS, call.info) end - isa(tv, TypeVar) || return CallMeta(Any, EFFECTS_THROWS, NoCallInfo()) + isa(tv, TypeVar) || return CallMeta(Any, EFFECTS_THROWS, call.info) body = UnionAll(tv, body) end ret = canconst ? Const(body) : Type{body} - return CallMeta(ret, Effects(EFFECTS_TOTAL; nothrow), NoCallInfo()) + return CallMeta(ret, Effects(EFFECTS_TOTAL; nothrow), call.info) end return CallMeta(Bottom, EFFECTS_THROWS, NoCallInfo()) end @@ -1998,12 +1998,20 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f), elseif la == 3 ub_var = argtypes[3] end + # make sure generic code is prepared for inlining if needed later + call = let T = Any[Type{TypeVar}, Any, Any, Any] + resize!(T, la) + atype = Tuple{T...} + T[1] = Const(TypeVar) + abstract_call_gf_by_type(interp, f, ArgInfo(nothing, T), si, atype, sv, max_methods) + end pT = typevar_tfunc(š•ƒįµ¢, n, lb_var, ub_var) effects = builtin_effects(š•ƒįµ¢, Core._typevar, ArgInfo(nothing, Any[Const(Core._typevar), n, lb_var, ub_var]), pT) - return CallMeta(pT, effects, NoCallInfo()) + return CallMeta(pT, effects, call.info) elseif f === UnionAll - return abstract_call_unionall(interp, argtypes) + call = abstract_call_gf_by_type(interp, f, ArgInfo(nothing, Any[Const(UnionAll), Any, Any]), si, Tuple{Type{UnionAll}, Any, Any}, sv, max_methods) + return abstract_call_unionall(interp, argtypes, call) elseif f === Tuple && la == 2 aty = argtypes[2] ty = isvarargtype(aty) ? unwrapva(aty) : widenconst(aty) @@ -2021,13 +2029,14 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f), end elseif la == 3 && istopfunction(f, :!==) # mark !== as exactly a negated call to === + call = abstract_call_gf_by_type(interp, f, ArgInfo(fargs, Any[Const(f), Any, Any]), si, Tuple{typeof(f), Any, Any}, sv, max_methods) rty = abstract_call_known(interp, (===), arginfo, si, sv, max_methods).rt if isa(rty, Conditional) return CallMeta(Conditional(rty.slot, rty.elsetype, rty.thentype), EFFECTS_TOTAL, NoCallInfo()) # swap if-else elseif isa(rty, Const) return CallMeta(Const(rty.val === false), EFFECTS_TOTAL, MethodResultPure()) end - return CallMeta(rty, EFFECTS_TOTAL, NoCallInfo()) + return call elseif la == 3 && istopfunction(f, :(>:)) # mark issupertype as a exact alias for issubtype # swap T1 and T2 arguments and call <: diff --git a/base/essentials.jl b/base/essentials.jl index 0d32116514052..db47f1ee7f2c1 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -219,7 +219,7 @@ macro _foldable_meta() #=:nothrow=#false, #=:terminates_globally=#true, #=:terminates_locally=#false, - #=:notaskstate=#false, + #=:notaskstate=#true, #=:inaccessiblememonly=#true, #=:noub=#true)) end @@ -264,6 +264,9 @@ end macro _propagate_inbounds_meta() return Expr(:meta, :inline, :propagate_inbounds) end +macro _nospecializeinfer_meta() + return Expr(:meta, :nospecializeinfer) +end function iterate end diff --git a/base/promotion.jl b/base/promotion.jl index 6e32bd7a42efa..bc081b15c7a31 100644 --- a/base/promotion.jl +++ b/base/promotion.jl @@ -18,10 +18,11 @@ Number ``` """ typejoin() = Bottom -typejoin(@nospecialize(t)) = t -typejoin(@nospecialize(t), ts...) = (@_foldable_meta; typejoin(t, typejoin(ts...))) +typejoin(@nospecialize(t)) = (@_nospecializeinfer_meta; t) +typejoin(@nospecialize(t), ts...) = (@_foldable_meta; @_nospecializeinfer_meta; typejoin(t, typejoin(ts...))) function typejoin(@nospecialize(a), @nospecialize(b)) @_foldable_meta + @_nospecializeinfer_meta if isa(a, TypeVar) return typejoin(a.ub, b) elseif isa(b, TypeVar) @@ -90,9 +91,9 @@ function typejoin(@nospecialize(a), @nospecialize(b)) elseif b <: Tuple return Any end - while b !== Any + while !(b === Any) if a <: b.name.wrapper - while a.name !== b.name + while !(a.name === b.name) a = supertype(a)::DataType end if a.name === Type.body.name @@ -139,6 +140,7 @@ end # (Core.Compiler.isnotbrokensubtype), use only simple types for `b` function typesplit(@nospecialize(a), @nospecialize(b)) @_foldable_meta + @_nospecializeinfer_meta if a <: b return Bottom end @@ -239,7 +241,8 @@ function full_va_len(p::Core.SimpleVector) end # reduce typejoin over A[i:end] -function tailjoin(A, i) +function tailjoin(A::SimpleVector, i::Int) + @_foldable_meta if i > length(A) return unwrapva(A[end]) end diff --git a/base/reflection.jl b/base/reflection.jl index 25ac2fbb74bee..12595f238e564 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -674,7 +674,7 @@ end iskindtype(@nospecialize t) = (t === DataType || t === UnionAll || t === Union || t === typeof(Bottom)) isconcretedispatch(@nospecialize t) = isconcretetype(t) && !iskindtype(t) -has_free_typevars(@nospecialize(t)) = ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0 +has_free_typevars(@nospecialize(t)) = (@_total_meta; ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0) # equivalent to isa(v, Type) && isdispatchtuple(Tuple{v}) || v === Union{} # and is thus perhaps most similar to the old (pre-1.0) `isleaftype` query diff --git a/base/tuple.jl b/base/tuple.jl index 59fe2c1e531e1..f01d2655da7c0 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -199,41 +199,31 @@ first(t::Tuple) = t[1] # eltype eltype(::Type{Tuple{}}) = Bottom -function eltype(t::Type{<:Tuple{Vararg{E}}}) where {E} - if @isdefined(E) - return E - else - # TODO: need to guard against E being miscomputed by subtyping (ref #23017) - # and compute the result manually in this case - return _compute_eltype(t) - end -end +# the <: here makes the runtime a bit more complicated (needing to check isdefined), but really helps inference +eltype(t::Type{<:Tuple{Vararg{E}}}) where {E} = @isdefined(E) ? (E isa Type ? E : Union{}) : _compute_eltype(t) eltype(t::Type{<:Tuple}) = _compute_eltype(t) -function _tuple_unique_fieldtypes(@nospecialize t) +function _compute_eltype(@nospecialize t) @_total_meta - types = IdSet() + has_free_typevars(t) && return Any tĀ“ = unwrap_unionall(t) # Given t = Tuple{Vararg{S}} where S<:Real, the various # unwrapping/wrapping/va-handling here will return Real if tĀ“ isa Union - union!(types, _tuple_unique_fieldtypes(rewrap_unionall(tĀ“.a, t))) - union!(types, _tuple_unique_fieldtypes(rewrap_unionall(tĀ“.b, t))) - else - for ti in (tĀ“::DataType).parameters - push!(types, rewrap_unionall(unwrapva(ti), t)) - end + return promote_typejoin(_compute_eltype(rewrap_unionall(tĀ“.a, t)), + _compute_eltype(rewrap_unionall(tĀ“.b, t))) end - return Core.svec(types...) -end -function _compute_eltype(@nospecialize t) - @_total_meta # TODO: the compiler shouldn't need this - types = _tuple_unique_fieldtypes(t) - return afoldl(types...) do a, b - # if we've already reached Any, it can't widen any more - a === Any && return Any - b === Any && return Any - return promote_typejoin(a, b) + p = (tĀ“::DataType).parameters + length(p) == 0 && return Union{} + elt = rewrap_unionall(unwrapva(p[1]), t) + elt isa Type || return Union{} # Tuple{2} is legal as a Type, but the eltype is Union{} since it is uninhabited + r = elt + for i in 2:length(p) + r === Any && return r # if we've already reached Any, it can't widen any more + elt = rewrap_unionall(unwrapva(p[i]), t) + elt isa Type || return Union{} # Tuple{2} is legal as a Type, but the eltype is Union{} since it is uninhabited + r = promote_typejoin(elt, r) end + return r end # We'd like to be able to infer eltype(::Tuple), which needs to be able to From 00c6ea777773b054e241c41e3c18f47c3704f905 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 30 Aug 2023 21:40:17 +0000 Subject: [PATCH 2/3] inference: teach tfunc that typeof tuples must have fixed lengths --- base/compiler/tfuncs.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index 3c103e56c5c03..23be0015b5951 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -731,8 +731,12 @@ function typeof_concrete_vararg(t::DataType) for i = 1:np p = t.parameters[i] if i == np && isvarargtype(p) - if isdefined(p, :T) && !isdefined(p, :N) && isconcretetype(p.T) - return Type{Tuple{t.parameters[1:np-1]..., Vararg{p.T, N}}} where N + if isdefined(p, :T) && isconcretetype(p.T) + t = Type{Tuple{t.parameters[1:np-1]..., Vararg{p.T, N}}} where N + if isdefined(p, :N) + return t{p.N} + end + return t end elseif !isconcretetype(p) break From 15f7db81eecc25d5967a8f784fbd20f0694ebcf1 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 28 Aug 2023 20:55:48 +0000 Subject: [PATCH 3/3] inference: fix bad effects for recursion Effects are not converged, so they need to always be correct, and not a bestguess, even during recursion. --- base/compiler/typeinfer.jl | 59 ++++++++++++++++------------ test/compiler/AbstractInterpreter.jl | 2 +- test/compiler/effects.jl | 19 ++++++--- test/compiler/inference.jl | 10 +++++ 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index f1ef36413303e..b629eff09611c 100644 --- a/base/compiler/typeinfer.jl +++ b/base/compiler/typeinfer.jl @@ -432,6 +432,34 @@ function cycle_fix_limited(@nospecialize(typ), sv::InferenceState) return typ end +function adjust_effects(ipo_effects::Effects, def::Method) + # override the analyzed effects using manually annotated effect settings + override = decode_effects_override(def.purity) + if is_effect_overridden(override, :consistent) + ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE) + end + if is_effect_overridden(override, :effect_free) + ipo_effects = Effects(ipo_effects; effect_free=ALWAYS_TRUE) + end + if is_effect_overridden(override, :nothrow) + ipo_effects = Effects(ipo_effects; nothrow=true) + end + if is_effect_overridden(override, :terminates_globally) + ipo_effects = Effects(ipo_effects; terminates=true) + end + if is_effect_overridden(override, :notaskstate) + ipo_effects = Effects(ipo_effects; notaskstate=true) + end + if is_effect_overridden(override, :inaccessiblememonly) + ipo_effects = Effects(ipo_effects; inaccessiblememonly=ALWAYS_TRUE) + end + if is_effect_overridden(override, :noub) + ipo_effects = Effects(ipo_effects; noub=ALWAYS_TRUE) + end + return ipo_effects +end + + function adjust_effects(sv::InferenceState) ipo_effects = sv.ipo_effects @@ -478,28 +506,7 @@ function adjust_effects(sv::InferenceState) # override the analyzed effects using manually annotated effect settings def = sv.linfo.def if isa(def, Method) - override = decode_effects_override(def.purity) - if is_effect_overridden(override, :consistent) - ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE) - end - if is_effect_overridden(override, :effect_free) - ipo_effects = Effects(ipo_effects; effect_free=ALWAYS_TRUE) - end - if is_effect_overridden(override, :nothrow) - ipo_effects = Effects(ipo_effects; nothrow=true) - end - if is_effect_overridden(override, :terminates_globally) - ipo_effects = Effects(ipo_effects; terminates=true) - end - if is_effect_overridden(override, :notaskstate) - ipo_effects = Effects(ipo_effects; notaskstate=true) - end - if is_effect_overridden(override, :inaccessiblememonly) - ipo_effects = Effects(ipo_effects; inaccessiblememonly=ALWAYS_TRUE) - end - if is_effect_overridden(override, :noub) - ipo_effects = Effects(ipo_effects; noub=ALWAYS_TRUE) - end + ipo_effects = adjust_effects(ipo_effects, def) end return ipo_effects @@ -850,8 +857,10 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize end typeinf(interp, frame) update_valid_age!(caller, frame.valid_worlds) - edge = is_inferred(frame) ? mi : nothing - return EdgeCallResult(frame.bestguess, edge, frame.ipo_effects) # effects are adjusted already within `finish` + isinferred = is_inferred(frame) + edge = isinferred ? mi : nothing + effects = isinferred ? frame.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_effects + return EdgeCallResult(frame.bestguess, edge, effects) elseif frame === true # unresolvable cycle return EdgeCallResult(Any, nothing, Effects()) @@ -859,7 +868,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize # return the current knowledge about this cycle frame = frame::InferenceState update_valid_age!(caller, frame.valid_worlds) - return EdgeCallResult(frame.bestguess, nothing, adjust_effects(frame)) + return EdgeCallResult(frame.bestguess, nothing, adjust_effects(Effects(), method)) end function cached_return_type(code::CodeInstance) diff --git a/test/compiler/AbstractInterpreter.jl b/test/compiler/AbstractInterpreter.jl index 2d7981f2567a5..403ba1d955fe1 100644 --- a/test/compiler/AbstractInterpreter.jl +++ b/test/compiler/AbstractInterpreter.jl @@ -134,7 +134,7 @@ function CC.concrete_eval_eligible(interp::Issue48097Interp, end @overlay Issue48097MT @noinline Core.throw_inexacterror(f::Symbol, ::Type{T}, val) where {T} = return issue48097(; kwargs...) = return 42 -@test fully_eliminated(; interp=Issue48097Interp(), retval=42) do +@test_broken fully_eliminated(; interp=Issue48097Interp(), retval=42) do issue48097(; a=1f0, b=1.0) end diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index be3e4da9eee37..ea9bc24343715 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -89,10 +89,14 @@ Base.@assume_effects :terminates_globally function recur_termination1(x) 0 ā‰¤ x < 20 || error("bad fact") return x * recur_termination1(x-1) end -@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,))) -@test fully_eliminated() do +@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,))) +@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination1, (Int,))) +function recur_termination2() + Base.@assume_effects :total !:terminates_globally recur_termination1(12) end +@test_broken fully_eliminated(recur_termination2) +@test fully_eliminated() do; recur_termination2(); end Base.@assume_effects :terminates_globally function recur_termination21(x) x == 0 && return 1 @@ -100,11 +104,16 @@ Base.@assume_effects :terminates_globally function recur_termination21(x) return recur_termination22(x) end recur_termination22(x) = x * recur_termination21(x-1) -@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,))) -@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,))) -@test fully_eliminated() do +@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,))) +@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,))) +@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination21, (Int,))) +@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination22, (Int,))) +function recur_termination2x() + Base.@assume_effects :total !:terminates_globally recur_termination21(12) + recur_termination22(12) end +@test_broken fully_eliminated(recur_termination2x) +@test fully_eliminated() do; recur_termination2x(); end # anonymous function support for `@assume_effects` @test fully_eliminated() do diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 152d806ccea85..08415f38c0121 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -5172,3 +5172,13 @@ end |> only === Val{5} @test fully_eliminated() do length(continue_const_prop(1, 5)) end + +# issue #51090 +@noinline function bar51090(b) + b == 0 && return + r = foo51090(b - 1) + Base.donotdelete(b) + return r +end +foo51090(b) = return bar51090(b) +@test !fully_eliminated(foo51090, (Int,))