From cd25161cfc238265311c05e994f8b858ef7edb51 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Sat, 15 Jul 2023 18:47:37 +0900 Subject: [PATCH] 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 --- base/compiler/ssair/inlining.jl | 2 +- base/compiler/ssair/passes.jl | 6 +++--- base/essentials.jl | 2 +- src/builtins.c | 8 +++----- test/compiler/inference.jl | 16 ++++++++++++++++ test/core.jl | 3 +++ 6 files changed, 27 insertions(+), 10 deletions(-) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index add390f7d454f6..cdbfaf64856993 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -1808,7 +1808,7 @@ end function insert_spval!(insert_node!::Inserter, spvals_ssa::SSAValue, spidx::Int, do_isdefined::Bool) ret = insert_node!( - effect_free_and_nothrow(NewInstruction(Expr(:call, Core._svec_ref, false, spvals_ssa, spidx), Any))) + effect_free_and_nothrow(NewInstruction(Expr(:call, Core._svec_ref, spvals_ssa, spidx), Any))) tcheck_not = nothing if do_isdefined tcheck = insert_node!( diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 638814fd0bc027..96a413e86778b5 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -811,10 +811,10 @@ function perform_lifting!(compact::IncrementalCompact, end function lift_svec_ref!(compact::IncrementalCompact, idx::Int, stmt::Expr) - length(stmt.args) != 4 && return + length(stmt.args) != 3 && return - vec = stmt.args[3] - val = stmt.args[4] + vec = stmt.args[2] + val = stmt.args[3] valT = argextype(val, compact) (isa(valT, Const) && isa(valT.val, Int)) || return valI = valT.val::Int diff --git a/base/essentials.jl b/base/essentials.jl index 7b70c0dff074d4..68dd0c06d646f4 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -761,7 +761,7 @@ end # SimpleVector -@eval getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref($(Expr(:boundscheck)), v, i)) +getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref(v, i)) function length(v::SimpleVector) @_total_meta t = @_gc_preserve_begin v diff --git a/src/builtins.c b/src/builtins.c index b664b8d73710fa..4f75fd79eadccb 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1655,11 +1655,9 @@ JL_CALLABLE(jl_f__compute_sparams) JL_CALLABLE(jl_f__svec_ref) { - JL_NARGS(_svec_ref, 3, 3); - jl_value_t *b = args[0]; - jl_svec_t *s = (jl_svec_t*)args[1]; - jl_value_t *i = (jl_value_t*)args[2]; - JL_TYPECHK(_svec_ref, bool, b); + JL_NARGS(_svec_ref, 2, 2); + jl_svec_t *s = (jl_svec_t*)args[0]; + jl_value_t *i = (jl_value_t*)args[1]; JL_TYPECHK(_svec_ref, simplevector, (jl_value_t*)s); JL_TYPECHK(_svec_ref, long, i); size_t len = jl_svec_len(s); diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index ded94380377333..cb16cb604881b9 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -5014,3 +5014,19 @@ let 𝕃 = Core.Compiler.SimpleInferenceLattice.instance map(T -> Issue49785{S,T}, (a = S,)) end isa Vector end + +# `getindex(::SimpleVector, ::Int)` shuold be concrete-evaluated +@eval Base.return_types() do + $(Core.svec(1,Int,nothing))[2] +end |> only == Type{Int} +# https://github.com/JuliaLang/julia/issues/50544 +struct Issue50544{T<:Tuple} + t::T +end +Base.@propagate_inbounds f_issue50544(x, i, ii...) = f_issue50544(f_issue50544(x, i), ii...) +Base.@propagate_inbounds f_issue50544(::Type{Issue50544{T}}, i) where T = T.parameters[i] +g_issue50544(T...) = Issue50544{Tuple{T...}} +h_issue50544(x::T) where T = g_issue50544(f_issue50544(T, 1), f_issue50544(T, 2, 1)) +let x = Issue50544((1, Issue50544((2.0, 'x')))) + @test only(Base.return_types(h_issue50544, (typeof(x),))) == Type{Issue50544{Tuple{Int,Float64}}} +end diff --git a/test/core.jl b/test/core.jl index a87c45b698e491..07cd2c95cf2846 100644 --- a/test/core.jl +++ b/test/core.jl @@ -8040,3 +8040,6 @@ bar50293(@nospecialize(u)) = (Base.issingletontype(u.a), baz50293(u.a)) let u = Union{Type{Union{}}, Type{Any}}, ab = bar50293(u) @test ab[1] == ab[2] == false end + +# `getindex(::SimpleVector, ::Int)` should be concrete-eval eligible +@test Core.Compiler.is_foldable(Base.infer_effects(getindex, (Core.SimpleVector,Int)))