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

RFC: A path forward on --check-bounds #50239

Closed
wants to merge 4 commits into from
Closed
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
3 changes: 3 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ if isdefined(Core, :Compiler) && is_primary_base_module
Docs.loaddocs(Core.Compiler.CoreDocs.DOCS)
end

# Formerly PrecompileTools
include("invalidations.jl")

# finally, now make `include` point to the full version
for m in methods(include)
delete_method(m)
Expand Down
4 changes: 2 additions & 2 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1017,9 +1017,9 @@ Dict{String, Int64} with 2 entries:
function setindex! end

@eval setindex!(A::Array{T}, x, i1::Int) where {T} =
arrayset($(Expr(:boundscheck)), A, x isa T ? x : convert(T,x)::T, i1)
arrayset(Core.should_check_bounds($(Expr(:boundscheck))), A, x isa T ? x : convert(T,x)::T, i1)
@eval setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T} =
(@inline; arrayset($(Expr(:boundscheck)), A, x isa T ? x : convert(T,x)::T, i1, i2, I...))
(@inline; arrayset(Core.should_check_bounds($(Expr(:boundscheck))), A, x isa T ? x : convert(T,x)::T, i1, i2, I...))

__inbounds_setindex!(A::Array{T}, x, i1::Int) where {T} =
arrayset(false, A, convert(T,x)::T, i1)
Expand Down
2 changes: 2 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -854,4 +854,6 @@ function _hasmethod(@nospecialize(tt)) # this function has a special tfunc
return Intrinsics.not_int(ccall(:jl_gf_invoke_lookup, Any, (Any, Any, UInt), tt, nothing, world) === nothing)
end

should_check_bounds(boundscheck::Bool) = boundscheck

ccall(:jl_set_istopmod, Cvoid, (Any, Bool), Core, true)
12 changes: 12 additions & 0 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,18 @@ function exec_options(opts)
invokelatest(Main.Distributed.process_opts, opts)
end

# Maybe redefine bounds checking if requested
if ccall(:jl_generating_output, Cint, ()) == 0
# Inoperative during output generation. Determined by mandatory deps mechanism.
if JLOptions().check_bounds != 0
if JLOptions().check_bounds == 1
require(PkgId(UUID((0xb3a877e5_8181_4b4a, 0x8173_0b9cb13136fe)),"--check-bounds=yes"))
else
require(PkgId(UUID((0x5ece1bc4_2007_43a8, 0xac47_40059be74678)),"--check-bounds=no"))
end
end
end

interactiveinput = (repl || is_interactive::Bool) && isa(stdin, TTY)
is_interactive::Bool |= interactiveinput

Expand Down
7 changes: 0 additions & 7 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -835,13 +835,6 @@ end
function concrete_eval_eligible(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::AbsIntState)
(;effects) = result
if inbounds_option() === :off
if !is_nothrow(effects)
# Disable concrete evaluation in `--check-bounds=no` mode,
# unless it is known to not throw.
return :none
end
end
if !effects.noinbounds && stmt_taints_inbounds_consistency(sv)
# If the current statement is @inbounds or we propagate inbounds, the call's consistency
# is tainted and not consteval eligible.
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,8 @@ function batch_inline!(ir::IRCode, todo::Vector{Pair{Int,Any}}, propagate_inboun
end
finish_cfg_inline!(state)

boundscheck = inbounds_option()
if boundscheck === :default && propagate_inbounds
boundscheck = :default
if propagate_inbounds
boundscheck = :propagate
end

Expand Down
6 changes: 0 additions & 6 deletions base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,3 @@ function coverage_enabled(m::Module)
end
return false
end
function inbounds_option()
opt_check_bounds = JLOptions().check_bounds
opt_check_bounds == 0 && return :default
opt_check_bounds == 1 && return :on
return :off
end
8 changes: 4 additions & 4 deletions base/essentials.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

import Core: CodeInfo, SimpleVector, donotdelete, compilerbarrier, arrayref
import Core: CodeInfo, SimpleVector, donotdelete, compilerbarrier, arrayref, should_check_bounds

const Callable = Union{Function,Type}

Expand All @@ -10,8 +10,8 @@ const Bottom = Union{}
length(a::Array) = arraylen(a)

# This is more complicated than it needs to be in order to get Win64 through bootstrap
eval(:(getindex(A::Array, i1::Int) = arrayref($(Expr(:boundscheck)), A, i1)))
eval(:(getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@inline; arrayref($(Expr(:boundscheck)), A, i1, i2, I...))))
eval(:(getindex(A::Array, i1::Int) = arrayref(should_check_bounds($(Expr(:boundscheck))), A, i1)))
eval(:(getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@inline; arrayref(should_check_bounds($(Expr(:boundscheck))), A, i1, i2, I...))))

==(a::GlobalRef, b::GlobalRef) = a.mod === b.mod && a.name === b.name

Expand Down Expand Up @@ -698,7 +698,7 @@ julia> f2()
implementation after you are certain its behavior is correct.
"""
macro boundscheck(blk)
return Expr(:if, Expr(:boundscheck), esc(blk))
return Expr(:if, Expr(:call, GlobalRef(Core, :should_check_bounds), Expr(:boundscheck)), esc(blk))
end

"""
Expand Down
4 changes: 2 additions & 2 deletions base/experimental.jl
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ Base.IndexStyle(::Type{<:Const}) = IndexLinear()
Base.size(C::Const) = size(C.a)
Base.axes(C::Const) = axes(C.a)
@eval Base.getindex(A::Const, i1::Int) =
(Base.@inline; Core.const_arrayref($(Expr(:boundscheck)), A.a, i1))
(Base.@inline; Core.const_arrayref(Core.should_check_bounds($(Expr(:boundscheck))), A.a, i1))
@eval Base.getindex(A::Const, i1::Int, i2::Int, I::Int...) =
(Base.@inline; Core.const_arrayref($(Expr(:boundscheck)), A.a, i1, i2, I...))
(Base.@inline; Core.const_arrayref(Core.should_check_bounds($(Expr(:boundscheck))), A.a, i1, i2, I...))

"""
@aliasscope expr
Expand Down
70 changes: 70 additions & 0 deletions base/invalidations.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
"""
@recompile_invalidations begin
using PkgA
end

Recompile any invalidations that occur within the given expression. This is generally intended to be used
by users in creating "Startup" packages to ensure that the code compiled by package authors is not invalidated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
by users in creating "Startup" packages to ensure that the code compiled by package authors is not invalidated.
by users in creating "Startup" packages to ensure that the code compiled by package authors is not invalidated.
!!! note
Users should use `PrecompileTools.jl`. This macro is solely for internal usage in the Julia bootstrapping process.

"""
macro recompile_invalidations(expr)
list = gensym(:list)
Expr(:toplevel,
:($list = ccall(:jl_debug_method_invalidation, Any, (Cint,), 1)),
Expr(:tryfinally,
esc(expr),
:(ccall(:jl_debug_method_invalidation, Any, (Cint,), 0))
),
:(if ccall(:jl_generating_output, Cint, ()) == 1
foreach($precompile_mi, $invalidation_leaves($list))
end)
)
end

function precompile_mi(mi)
precompile(mi.specTypes) # TODO: Julia should allow one to pass `mi` directly (would handle `invoke` properly)
return
end

function invalidation_leaves(invlist)
umis = Set{Core.MethodInstance}()
# `queued` is a queue of length 0 or 1 of invalidated MethodInstances.
# We wait to read the `depth` to find out if it's a leaf.
queued, depth = nothing, 0
function cachequeued(item, nextdepth)
if queued !== nothing && nextdepth <= depth
push!(umis, queued)
end
queued, depth = item, nextdepth
end

i, ilast = firstindex(invlist), lastindex(invlist)
while i <= ilast
item = invlist[i]
if isa(item, Core.MethodInstance)
if i < lastindex(invlist)
nextitem = invlist[i+1]
if nextitem == "invalidate_mt_cache"
cachequeued(nothing, 0)
i += 2
continue
end
if nextitem ∈ ("jl_method_table_disable", "jl_method_table_insert", "verify_methods")
cachequeued(nothing, 0)
push!(umis, item)
end
if isa(nextitem, Integer)
cachequeued(item, nextitem)
i += 2
continue
end
end
end
if (isa(item, Method) || isa(item, Type)) && queued !== nothing
push!(umis, queued)
queued, depth = nothing, 0
end
i += 1
end
return umis
end
Loading