Skip to content

Commit e730590

Browse files
committed
sparam inlining: Handle undefined sparams
`_compute_sparams` is not required to be able to resolve an sparam, it can leave it as a `TypeVar`, in which case accessing it should throw an UndefVarError. This needs to be appropriately handled in inlining. We have a test case like this in the test suite, but it was being papered over by an incorrect check in can_inline_typevar. Remove that check and implement proper undef checking in inlining.
1 parent 5d9807d commit e730590

File tree

3 files changed

+60
-14
lines changed

3 files changed

+60
-14
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,10 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
361361
if extra_coverage_line != 0
362362
insert_node_here!(compact, NewInstruction(Expr(:code_coverage_effect), Nothing, extra_coverage_line))
363363
end
364+
sp_ssa = nothing
364365
if !validate_sparams(sparam_vals)
365366
# N.B. This works on the caller-side argexprs, (i.e. before the va fixup below)
366-
sparam_vals = insert_node_here!(compact,
367+
sp_ssa = insert_node_here!(compact,
367368
effect_free(NewInstruction(Expr(:call, Core._compute_sparams, def, argexprs...), SimpleVector, topline)))
368369
end
369370
if def.isva
@@ -398,7 +399,7 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
398399
# face of rename_arguments! mutating in place - should figure out
399400
# something better eventually.
400401
inline_compact[idx′] = nothing
401-
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, sig, sparam_vals, linetable_offset, boundscheck, inline_compact)
402+
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, sig, sparam_vals, sp_ssa, linetable_offset, boundscheck, inline_compact)
402403
if isa(stmt′, ReturnNode)
403404
val = stmt′.val
404405
return_value = SSAValue(idx′)
@@ -425,7 +426,7 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
425426
inline_compact = IncrementalCompact(compact, spec.ir, compact.result_idx)
426427
for ((_, idx′), stmt′) in inline_compact
427428
inline_compact[idx′] = nothing
428-
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, sig, sparam_vals, linetable_offset, boundscheck, inline_compact)
429+
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, sig, sparam_vals, sp_ssa, linetable_offset, boundscheck, inline_compact)
429430
if isa(stmt′, ReturnNode)
430431
if isdefined(stmt′, :val)
431432
val = stmt′.val
@@ -903,7 +904,6 @@ end
903904

904905
function can_inline_typevars(method::Method, argtypes::Vector{Any})
905906
may_have_fcalls(method) && return false
906-
any(@nospecialize(x) -> x isa UnionAll, argtypes[2:end]) && return false
907907
return true
908908
end
909909
can_inline_typevars(m::MethodMatch, argtypes::Vector{Any}) = can_inline_typevars(m.method, argtypes)
@@ -1724,15 +1724,30 @@ function late_inline_special_case!(
17241724
end
17251725

17261726
function ssa_substitute!(idx::Int, @nospecialize(val), arg_replacements::Vector{Any},
1727-
@nospecialize(spsig), spvals::Union{SimpleVector, SSAValue},
1727+
@nospecialize(spsig), spvals::SimpleVector,
1728+
spvals_ssa::Union{Nothing, SSAValue},
17281729
linetable_offset::Int32, boundscheck::Symbol, compact::IncrementalCompact)
17291730
compact.result[idx][:flag] &= ~IR_FLAG_INBOUNDS
17301731
compact.result[idx][:line] += linetable_offset
1731-
return ssa_substitute_op!(val, arg_replacements, spsig, spvals, boundscheck, compact, idx)
1732+
return ssa_substitute_op!(val, arg_replacements, spsig, spvals, spvals_ssa, boundscheck, compact, idx)
1733+
end
1734+
1735+
function insert_spval!(compact::IncrementalCompact, idx::Int, spvals_ssa::SSAValue, spidx::Int, do_isdefined::Bool)
1736+
ret = insert_node!(compact, SSAValue(idx),
1737+
effect_free(NewInstruction(Expr(:call, Core._svec_ref, false, spvals_ssa, spidx), Any)))
1738+
tcheck_not = nothing
1739+
if do_isdefined
1740+
tcheck = insert_node!(compact, SSAValue(idx),
1741+
effect_free(NewInstruction(Expr(:call, Core.isa, ret, Core.TypeVar), Bool)))
1742+
tcheck_not = insert_node!(compact, SSAValue(idx),
1743+
effect_free(NewInstruction(Expr(:call, not_int, tcheck), Bool)))
1744+
end
1745+
return (ret, tcheck_not)
17321746
end
17331747

17341748
function ssa_substitute_op!(@nospecialize(val), arg_replacements::Vector{Any},
1735-
@nospecialize(spsig), spvals::Union{SimpleVector, SSAValue},
1749+
@nospecialize(spsig), spvals::SimpleVector,
1750+
spvals_ssa::Union{Nothing, SSAValue},
17361751
boundscheck::Symbol, compact::IncrementalCompact, idx::Int)
17371752
if isa(val, Argument)
17381753
return arg_replacements[val.n]
@@ -1741,20 +1756,36 @@ function ssa_substitute_op!(@nospecialize(val), arg_replacements::Vector{Any},
17411756
e = val::Expr
17421757
head = e.head
17431758
if head === :static_parameter
1744-
if isa(spvals, SimpleVector)
1745-
return quoted(spvals[e.args[1]::Int])
1759+
spidx = e.args[1]::Int
1760+
val = spvals[spidx]
1761+
if !isa(val, TypeVar) && val !== Vararg
1762+
return quoted(val)
17461763
else
1747-
ret = insert_node!(compact, SSAValue(idx),
1748-
effect_free(NewInstruction(Expr(:call, Core._svec_ref, false, spvals, e.args[1]), Any)))
1764+
flag = compact[SSAValue(idx)][:flag]
1765+
maybe_undef = (flag & IR_FLAG_NOTHROW) == 0 && isa(val, TypeVar)
1766+
(ret, tcheck_not) = insert_spval!(compact, idx, spvals_ssa, spidx, maybe_undef)
1767+
if maybe_undef
1768+
insert_node!(compact, SSAValue(idx),
1769+
non_effect_free(NewInstruction(Expr(:throw_undef_if_not, val.name, tcheck_not), Nothing)))
1770+
end
17491771
return ret
17501772
end
1751-
elseif head === :cfunction && isa(spvals, SimpleVector)
1773+
elseif head === :isdefined && isa(e.args[1], Expr) && e.args[1].head === :static_parameter
1774+
spidx = (e.args[1]::Expr).args[1]::Int
1775+
val = spvals[spidx]
1776+
if !isa(val, TypeVar)
1777+
return true
1778+
else
1779+
(_, tcheck_not) = insert_spval!(compact, idx, spvals_ssa, spidx, true)
1780+
return tcheck_not
1781+
end
1782+
elseif head === :cfunction && spvals_ssa === nothing
17521783
@assert !isa(spsig, UnionAll) || !isempty(spvals)
17531784
e.args[3] = ccall(:jl_instantiate_type_in_env, Any, (Any, Any, Ptr{Any}), e.args[3], spsig, spvals)
17541785
e.args[4] = svec(Any[
17551786
ccall(:jl_instantiate_type_in_env, Any, (Any, Any, Ptr{Any}), argt, spsig, spvals)
17561787
for argt in e.args[4]::SimpleVector ]...)
1757-
elseif head === :foreigncall && isa(spvals, SimpleVector)
1788+
elseif head === :foreigncall && spvals_ssa === nothing
17581789
@assert !isa(spsig, UnionAll) || !isempty(spvals)
17591790
for i = 1:length(e.args)
17601791
if i == 2
@@ -1778,7 +1809,7 @@ function ssa_substitute_op!(@nospecialize(val), arg_replacements::Vector{Any},
17781809
isa(val, Union{SSAValue, NewSSAValue}) && return val # avoid infinite loop
17791810
urs = userefs(val)
17801811
for op in urs
1781-
op[] = ssa_substitute_op!(op[], arg_replacements, spsig, spvals, boundscheck, compact, idx)
1812+
op[] = ssa_substitute_op!(op[], arg_replacements, spsig, spvals, spvals_ssa, boundscheck, compact, idx)
17821813
end
17831814
return urs[]
17841815
end

test/compiler/inline.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,3 +1512,12 @@ end
15121512
# Test getfield modeling of Type{Ref{_A}} where _A
15131513
@test Core.Compiler.getfield_tfunc(Type, Core.Compiler.Const(:parameters)) !== Union{}
15141514
@test fully_eliminated(Base.ismutable, Tuple{Base.RefValue})
1515+
1516+
# TODO: Remove compute sparams for vararg_retrival
1517+
fvarargN_inline(x::Tuple{Vararg{Int, N}}) where {N} = N
1518+
fvarargN_inline(args...) = fvarargN_inline(args)
1519+
let src = code_typed1(fvarargN_inline, (Tuple{Vararg{Int}},))
1520+
@test_broken count(iscall((src, Core._compute_sparams)), src.code) == 0 &&
1521+
count(iscall((src, Core._svec_ref)), src.code) == 0 &&
1522+
count(iscall((src, Core.nfields)), src.code) == 1
1523+
end

test/core.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7831,3 +7831,9 @@ end
78317831
# Correct isdefined error for isdefined of Module of Int fld
78327832
f_isdefined_one(@nospecialize(x)) = isdefined(x, 1)
78337833
@test (try; f_isdefined_one(@__MODULE__); catch err; err; end).got === 1
7834+
7835+
# Unspecialized retrieval of vararg length
7836+
fvarargN(x::Tuple{Vararg{Int, N}}) where {N} = N
7837+
fvarargN(args...) = fvarargN(args)
7838+
finvokevarargN() = Base.inferencebarrier(fvarargN)(1, 2, 3)
7839+
@test finvokevarargN() == 3

0 commit comments

Comments
 (0)