Skip to content

Commit

Permalink
exclude the first builtin function from argtypes of builtin_effects
Browse files Browse the repository at this point in the history
Streamlined the `builtin_effects` function to accept `argtypes` without
including the builtin function itself. The first argument in the
previous implementation of `argtypes` was just redundant. This change
might reduce unnecessary allocations.
  • Loading branch information
aviatesk committed Nov 16, 2023
1 parent 3c82403 commit 574c5d1
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 30 deletions.
4 changes: 3 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2017,7 +2017,9 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
return abstract_applicable(interp, argtypes, sv, max_methods)
end
rt = abstract_call_builtin(interp, f, arginfo, sv)
ft = popfirst!(argtypes)
effects = builtin_effects(𝕃ᡒ, f, argtypes, rt)
pushfirst!(argtypes, ft)
return CallMeta(rt, effects, NoCallInfo())
elseif isa(f, Core.OpaqueClosure)
# calling an OpaqueClosure about which we have no information returns no information
Expand All @@ -2043,7 +2045,7 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
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, Any[Const(Core._typevar), n, lb_var, ub_var], pT)
effects = builtin_effects(𝕃ᡒ, Core._typevar, Any[n, lb_var, ub_var], pT)
return CallMeta(pT, effects, call.info)
elseif f === UnionAll
call = abstract_call_gf_by_type(interp, f, ArgInfo(nothing, Any[Const(UnionAll), Any, Any]), si, Tuple{Type{UnionAll}, Any, Any}, sv, max_methods)
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
isa(f, Builtin) || return (false, false, false)
# Needs to be handled in inlining to look at the callee effects
f === Core._apply_iterate && return (false, false, false)
argtypes = Any[argextype(args[arg], src) for arg in 1:length(args)]
argtypes = Any[argextype(args[arg], src) for arg in 2:length(args)]
effects = builtin_effects(𝕃ₒ, f, argtypes, rt)
consistent = is_consistent(effects)
effect_free = is_effect_free(effects)
Expand Down
52 changes: 30 additions & 22 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -923,16 +923,16 @@ function try_compute_fieldidx(@nospecialize(typ), @nospecialize(field))
end

function getfield_boundscheck(argtypes::Vector{Any})
if length(argtypes) == 3
if length(argtypes) == 2
return :on
elseif length(argtypes) == 4
boundscheck = argtypes[4]
elseif length(argtypes) == 3
boundscheck = argtypes[3]
isvarargtype(boundscheck) && return :unsafe
if widenconst(boundscheck) === Symbol
return :on
end
elseif length(argtypes) == 5
boundscheck = argtypes[5]
elseif length(argtypes) == 4
boundscheck = argtypes[4]
else
return :unsafe
end
Expand All @@ -949,16 +949,15 @@ end

function getfield_nothrow(𝕃::AbstractLattice, argtypes::Vector{Any}, boundscheck::Symbol=getfield_boundscheck(argtypes))
boundscheck === :unsafe && return false
(;argtypes) = arginfo
ordering = Const(:not_atomic)
if length(argtypes) == 4
isvarargtype(argtypes[4]) && return false
if widenconst(argtypes[4]) !== Bool
ordering = argtypes[4]
end
elseif length(argtypes) == 5
ordering = argtypes[4]
elseif length(argtypes) != 3
if length(argtypes) == 3
isvarargtype(argtypes[3]) && return false
if widenconst(argtypes[3]) !== Bool
ordering = argtypes[3]
end
elseif length(argtypes) == 4
ordering = argtypes[3]
elseif length(argtypes) β‰  2
return false
end
isa(ordering, Const) || return false
Expand All @@ -967,7 +966,7 @@ function getfield_nothrow(𝕃::AbstractLattice, argtypes::Vector{Any}, boundsch
if ordering !== :not_atomic # TODO: this is assuming not atomic
return false
end
return getfield_nothrow(𝕃, argtypes[2], argtypes[3], !(boundscheck === :off))
return getfield_nothrow(𝕃, argtypes[1], argtypes[2], !(boundscheck === :off))
end
@nospecs function getfield_nothrow(𝕃::AbstractLattice, s00, name, boundscheck::Bool)
# If we don't have boundscheck off and don't know the field, don't even bother
Expand Down Expand Up @@ -2175,7 +2174,7 @@ end
elseif f === invoke
return false
elseif f === getfield
return getfield_nothrow(𝕃, Any[Const(f), argtypes...])
return getfield_nothrow(𝕃, argtypes)
elseif f === setfield!
if na == 3
return setfield!_nothrow(𝕃, argtypes[1], argtypes[2], argtypes[3])
Expand Down Expand Up @@ -2380,8 +2379,8 @@ function isdefined_effects(𝕃::AbstractLattice, argtypes::Vector{Any})
end

function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospecialize(rt))
length(argtypes) < 3 && return EFFECTS_THROWS
obj = argtypes[2]
length(argtypes) < 2 && return EFFECTS_THROWS
obj = argtypes[1]
if isvarargtype(obj)
return Effects(EFFECTS_TOTAL;
consistent=CONSISTENT_IF_INACCESSIBLEMEMONLY,
Expand All @@ -2399,7 +2398,7 @@ function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospeci
# with undefined value to avoid tainting `:consistent` too aggressively
# TODO this should probably taint `:noub`, however, it would hinder concrete eval for
# `REPLInterpreter` that can ignore `:consistent-cy`, causing worse completions
if !(length(argtypes) β‰₯ 3 && getfield_notundefined(obj, argtypes[3]))
if !(length(argtypes) β‰₯ 2 && getfield_notundefined(obj, argtypes[2]))
consistent = ALWAYS_FALSE
end
noub = ALWAYS_TRUE
Expand All @@ -2419,7 +2418,7 @@ function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospeci
end
end
if hasintersect(widenconst(obj), Module)
inaccessiblememonly = getglobal_effects(argtypes[2:end], rt).inaccessiblememonly
inaccessiblememonly = getglobal_effects(argtypes, rt).inaccessiblememonly
elseif is_mutation_free_argtype(obj)
inaccessiblememonly = ALWAYS_TRUE
else
Expand Down Expand Up @@ -2447,9 +2446,14 @@ function getglobal_effects(argtypes::Vector{Any}, @nospecialize(rt))
return Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly)
end

"""
builtin_effects(𝕃::AbstractLattice, f::Builtin, argtypes::Vector{Any}, rt) -> Effects
Compute the effects of a builtin function call. `argtypes` should not include `f` itself.
"""
function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argtypes::Vector{Any}, @nospecialize(rt))
if isa(f, IntrinsicFunction)
return intrinsic_effects(f, argtypes[2:end])
return intrinsic_effects(f, argtypes)
end

@assert !contains_is(_SPECIAL_BUILTINS, f)
Expand All @@ -2463,7 +2467,6 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
# note this is safe only if we accounted for :noub already
rt === Bottom && return EFFECTS_THROWS

argtypes = arginfo.argtypes[2:end]
if f === isdefined
return isdefined_effects(𝕃, argtypes)
elseif f === getglobal
Expand Down Expand Up @@ -2509,6 +2512,11 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
end
end

"""
builtin_nothrow(𝕃::AbstractLattice, f::Builtin, argtypes::Vector{Any}, rt) -> Bool
Compute throw-ness of a builtin function call. `argtypes` should not include `f` itself.
"""
function builtin_nothrow(𝕃::AbstractLattice, @nospecialize(f), argtypes::Vector{Any}, @nospecialize(rt))
rt === Bottom && return false
contains_is(_PURE_BUILTINS, f) && return true
Expand Down
4 changes: 2 additions & 2 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1794,8 +1794,8 @@ function infer_effects(@nospecialize(f), @nospecialize(types=default_tt(f));
error("code reflection cannot be used from generated functions")
if isa(f, Core.Builtin)
types = to_tuple_type(types)
argtypes = Any[Core.Const(f), types.parameters...]
rt = Core.Compiler.builtin_tfunction(interp, f, argtypes[2:end], nothing)
argtypes = Any[types.parameters...]
rt = Core.Compiler.builtin_tfunction(interp, f, argtypes, nothing)
return Core.Compiler.builtin_effects(Core.Compiler.typeinf_lattice(interp), f, argtypes, rt)
end
tt = signature_type(f, types)
Expand Down
8 changes: 4 additions & 4 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,9 @@ end |> Core.Compiler.is_effect_free
# `getfield_effects` handles access to union object nicely
let 𝕃 = Core.Compiler.fallback_lattice
getfield_effects = Core.Compiler.getfield_effects
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Core.Const(getfield), Some{String}, Core.Const(:value)], String))
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Core.Const(getfield), Some{Symbol}, Core.Const(:value)], Symbol))
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Core.Const(getfield), Union{Some{Symbol},Some{String}}, Core.Const(:value)], Union{Symbol,String}))
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Some{String}, Core.Const(:value)], String))
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Some{Symbol}, Core.Const(:value)], Symbol))
@test Core.Compiler.is_consistent(getfield_effects(𝕃, Any[Union{Some{Symbol},Some{String}}, Core.Const(:value)], Union{Symbol,String}))
end
@test Base.infer_effects((Bool,)) do c
obj = c ? Some{String}("foo") : Some{Symbol}(:bar)
Expand Down Expand Up @@ -881,7 +881,7 @@ end

# Test that builtin_effects handles vararg correctly
@test !Core.Compiler.is_nothrow(Core.Compiler.builtin_effects(Core.Compiler.fallback_lattice, Core.isdefined,
Any[Core.Compiler.Const(Core.isdefined), String, Vararg{Any}], Bool))
Any[String, Vararg{Any}], Bool))

# Test that :new can be eliminated even if an sparam is unknown
struct SparamUnused{T}
Expand Down

0 comments on commit 574c5d1

Please sign in to comment.