Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove :boundscheck argument from Core._svec_ref #50561

Merged
merged 1 commit into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
  • Loading branch information
aviatesk committed Jul 15, 2023
commit 94ad38e1e77850a9be8bbffc5286b7cc61c06f0d
2 changes: 1 addition & 1 deletion base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8040,3 +8040,7 @@ 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

# `SimpleVector`-operations should be concrete-eval eligible
@test Core.Compiler.is_foldable(Base.infer_effects(length, (Core.SimpleVector,)))
@test Core.Compiler.is_foldable(Base.infer_effects(getindex, (Core.SimpleVector,Int)))