Skip to content

Commit cd25161

Browse files
committed
remove :boundscheck argument from Core._svec_ref
`Core._svec_ref` has accepted `boundscheck`-value as the first argument since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls `jl_svec_ref` in either the interpreter or the codegen, and thus the `boundscheck` value isn't utilized in any optimizations. Rather, even worse, this `boundscheck`-argument negatively influences the effect analysis (xref #50167 for details) and has caused type inference regressions as reported in #50544. For these reasons, this commit simply eliminates the `boundscheck` argument from `Core._svec_ref`. Consequently, `getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible. closes #50544
1 parent 22ac24a commit cd25161

File tree

6 files changed

+27
-10
lines changed

6 files changed

+27
-10
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1808,7 +1808,7 @@ end
18081808

18091809
function insert_spval!(insert_node!::Inserter, spvals_ssa::SSAValue, spidx::Int, do_isdefined::Bool)
18101810
ret = insert_node!(
1811-
effect_free_and_nothrow(NewInstruction(Expr(:call, Core._svec_ref, false, spvals_ssa, spidx), Any)))
1811+
effect_free_and_nothrow(NewInstruction(Expr(:call, Core._svec_ref, spvals_ssa, spidx), Any)))
18121812
tcheck_not = nothing
18131813
if do_isdefined
18141814
tcheck = insert_node!(

base/compiler/ssair/passes.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,10 +811,10 @@ function perform_lifting!(compact::IncrementalCompact,
811811
end
812812

813813
function lift_svec_ref!(compact::IncrementalCompact, idx::Int, stmt::Expr)
814-
length(stmt.args) != 4 && return
814+
length(stmt.args) != 3 && return
815815

816-
vec = stmt.args[3]
817-
val = stmt.args[4]
816+
vec = stmt.args[2]
817+
val = stmt.args[3]
818818
valT = argextype(val, compact)
819819
(isa(valT, Const) && isa(valT.val, Int)) || return
820820
valI = valT.val::Int

base/essentials.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,7 @@ end
761761

762762
# SimpleVector
763763

764-
@eval getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref($(Expr(:boundscheck)), v, i))
764+
getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref(v, i))
765765
function length(v::SimpleVector)
766766
@_total_meta
767767
t = @_gc_preserve_begin v

src/builtins.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,11 +1655,9 @@ JL_CALLABLE(jl_f__compute_sparams)
16551655

16561656
JL_CALLABLE(jl_f__svec_ref)
16571657
{
1658-
JL_NARGS(_svec_ref, 3, 3);
1659-
jl_value_t *b = args[0];
1660-
jl_svec_t *s = (jl_svec_t*)args[1];
1661-
jl_value_t *i = (jl_value_t*)args[2];
1662-
JL_TYPECHK(_svec_ref, bool, b);
1658+
JL_NARGS(_svec_ref, 2, 2);
1659+
jl_svec_t *s = (jl_svec_t*)args[0];
1660+
jl_value_t *i = (jl_value_t*)args[1];
16631661
JL_TYPECHK(_svec_ref, simplevector, (jl_value_t*)s);
16641662
JL_TYPECHK(_svec_ref, long, i);
16651663
size_t len = jl_svec_len(s);

test/compiler/inference.jl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5014,3 +5014,19 @@ let 𝕃 = Core.Compiler.SimpleInferenceLattice.instance
50145014
map(T -> Issue49785{S,T}, (a = S,))
50155015
end isa Vector
50165016
end
5017+
5018+
# `getindex(::SimpleVector, ::Int)` shuold be concrete-evaluated
5019+
@eval Base.return_types() do
5020+
$(Core.svec(1,Int,nothing))[2]
5021+
end |> only == Type{Int}
5022+
# https://github.com/JuliaLang/julia/issues/50544
5023+
struct Issue50544{T<:Tuple}
5024+
t::T
5025+
end
5026+
Base.@propagate_inbounds f_issue50544(x, i, ii...) = f_issue50544(f_issue50544(x, i), ii...)
5027+
Base.@propagate_inbounds f_issue50544(::Type{Issue50544{T}}, i) where T = T.parameters[i]
5028+
g_issue50544(T...) = Issue50544{Tuple{T...}}
5029+
h_issue50544(x::T) where T = g_issue50544(f_issue50544(T, 1), f_issue50544(T, 2, 1))
5030+
let x = Issue50544((1, Issue50544((2.0, 'x'))))
5031+
@test only(Base.return_types(h_issue50544, (typeof(x),))) == Type{Issue50544{Tuple{Int,Float64}}}
5032+
end

test/core.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8040,3 +8040,6 @@ bar50293(@nospecialize(u)) = (Base.issingletontype(u.a), baz50293(u.a))
80408040
let u = Union{Type{Union{}}, Type{Any}}, ab = bar50293(u)
80418041
@test ab[1] == ab[2] == false
80428042
end
8043+
8044+
# `getindex(::SimpleVector, ::Int)` should be concrete-eval eligible
8045+
@test Core.Compiler.is_foldable(Base.infer_effects(getindex, (Core.SimpleVector,Int)))

0 commit comments

Comments
 (0)