RFC: A path forward on --check-bounds

In 1.9, `--check-bounds=no` has started causing significant performance
regressions (e.g. #50110). This is because we switched a number of functions that
used to be `@pure` to new effects-based infrastructure, which very closely tracks
the the legality conditions for concrete evaluation. Unfortunately, disabling
bounds checking completely invalidates all prior legality analysis, so the only
realistic path we have is to completely disable it.

In general, we are learning that these kinds of global make-things-faster-but-unsafe
flags are highly problematic for a language for several reasons:

- Code is written with the assumption of a particular mode being chosen, so
  it is in general not possible or unsafe to compose libraries (which in a language
  like julia is a huge problem).

- Unsafe semantics are often harder for the compiler to reason about, causing
  unexpected performance issues (although the 1.9 --check-bounds=no issues are
  worse than just disabling concrete eval for things that use bounds checking)

In general, I'd like to remove the `--check-bounds=` option entirely (#48245),
but that proposal has encountered major opposition.

This PR implements an alternative proposal: We introduce a new function
`Core.should_check_bounds(boundscheck::Bool) = boundscheck`. This function is
passed the result of `Expr(:boundscheck)` (which is now purely determined by
the inliner based on `@inbounds`, without regard for the command line flag).

In this proposal, what the command line flag does is simply redefine this
function to either `true` or `false` (unconditionally) depending on the
value of the flag.

Of course, this causes massive amounts of recompilation, but I think this can
be addressed by adding logic to loading that loads a pkgimage with appropriate
definitions to cure the invalidations. The special logic we have now now
to take account of the --check-bounds flag in .ji selection, would be replaced
by automatically injecting the special pkgimage as a dependency to every
loaded image. This part isn't implemented in this PR, but I think it's reasonable
to do.

I think with that, the `--check-bounds` flag remains functional, while having
much more well defined behavior, as it relies on the standard world age
mechanisms.

A major benefit of this approach is that it can be scoped appropriately using
overlay tables. For exmaple:

```
julia> using CassetteOverlay

julia> @MethodTable AssumeInboundsTable;

julia> @overlay AssumeInboundsTable Core.should_check_bounds(b::Bool) = false;

julia> assume_inbounds = @overlaypass AssumeInboundsTable

julia> assume_inbounds(f, args...) # f(args...) with bounds checking disabled dynamically
```

Similar logic applies to GPUCompiler, which already supports overlay tables.

Author: Keno

base/array.jl

@@ -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)

base/boot.jl

@@ -852,4 +852,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)

base/client.jl

@@ -272,6 +272,15 @@ function exec_options(opts)
         invokelatest(Main.Distributed.process_opts, opts)
     end
 
+    # Maybe redefine bounds checking if requested
+    if JLOptions().check_bounds != 0
+        if JLOptions().check_bounds == 1
+            Core.eval(Main, :(Core.should_check_bounds(boundscheck::Bool) = true))
+        else
+            Core.eval(Main, :(Core.should_check_bounds(boundscheck::Bool) = false))
+        end
+    end
+
     interactiveinput = (repl || is_interactive::Bool) && isa(stdin, TTY)
     is_interactive::Bool |= interactiveinput
 

base/compiler/abstractinterpretation.jl

@@ -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.

base/compiler/ssair/inlining.jl

@@ -664,8 +664,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
 

base/compiler/utilities.jl

@@ -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

base/essentials.jl

@@ -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}
 
@@ -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
 
@@ -678,7 +678,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
 
 """
@@ -741,7 +741,7 @@ end
 
 # SimpleVector
 
-@eval getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref($(Expr(:boundscheck)), v, i))
+@eval getindex(v::SimpleVector, i::Int) = (@_foldable_meta; Core._svec_ref(should_check_bounds($(Expr(:boundscheck))), v, i))
 function length(v::SimpleVector)
     @_total_meta
     t = @_gc_preserve_begin v

base/experimental.jl

@@ -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

base/tuple.jl

@@ -28,8 +28,8 @@ firstindex(@nospecialize t::Tuple) = 1
 lastindex(@nospecialize t::Tuple) = length(t)
 size(@nospecialize(t::Tuple), d::Integer) = (d == 1) ? length(t) : throw(ArgumentError("invalid tuple dimension $d"))
 axes(@nospecialize t::Tuple) = (OneTo(length(t)),)
-@eval getindex(@nospecialize(t::Tuple), i::Int) = getfield(t, i, $(Expr(:boundscheck)))
-@eval getindex(@nospecialize(t::Tuple), i::Integer) = getfield(t, convert(Int, i), $(Expr(:boundscheck)))
+@eval getindex(@nospecialize(t::Tuple), i::Int) = getfield(t, i, should_check_bounds($(Expr(:boundscheck))))
+@eval getindex(@nospecialize(t::Tuple), i::Integer) = getfield(t, convert(Int, i), should_check_bounds($(Expr(:boundscheck))))
 __inbounds_getindex(@nospecialize(t::Tuple), i::Int) = getfield(t, i, false)
 __inbounds_getindex(@nospecialize(t::Tuple), i::Integer) = getfield(t, convert(Int, i), false)
 getindex(t::Tuple, r::AbstractArray{<:Any,1}) = (eltype(t)[t[ri] for ri in r]...,)

src/cgutils.cpp

@@ -1736,26 +1736,10 @@ static void emit_concretecheck(jl_codectx_t &ctx, Value *typ, const std::string
     error_unless(ctx, emit_isconcrete(ctx, typ), msg);
 }
 
-#define CHECK_BOUNDS 1
-static bool bounds_check_enabled(jl_codectx_t &ctx, jl_value_t *inbounds) {
-#if CHECK_BOUNDS==1
-    if (jl_options.check_bounds == JL_OPTIONS_CHECK_BOUNDS_ON)
-        return 1;
-    if (jl_options.check_bounds == JL_OPTIONS_CHECK_BOUNDS_OFF)
-        return 0;
-    if (inbounds == jl_false)
-        return 0;
-    return 1;
-#else
-    return 0;
-#endif
-}
-
 static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_value_t *ty, Value *i, Value *len, jl_value_t *boundscheck)
 {
     Value *im1 = ctx.builder.CreateSub(i, ConstantInt::get(ctx.types().T_size, 1));
-#if CHECK_BOUNDS==1
-    if (bounds_check_enabled(ctx, boundscheck)) {
+    if (boundscheck != jl_false) {
         ++EmittedBoundschecks;
         Value *ok = ctx.builder.CreateICmpULT(im1, len);
         setName(ctx.emission_context, ok, "boundscheck");
@@ -1790,7 +1774,6 @@ static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_v
         ctx.f->getBasicBlockList().push_back(passBB);
         ctx.builder.SetInsertPoint(passBB);
     }
-#endif
     return im1;
 }
 
@@ -2960,14 +2943,12 @@ static Value *emit_array_nd_index(
     Value *a = boxed(ctx, ainfo);
     Value *i = Constant::getNullValue(ctx.types().T_size);
     Value *stride = ConstantInt::get(ctx.types().T_size, 1);
-#if CHECK_BOUNDS==1
-    bool bc = bounds_check_enabled(ctx, inbounds);
+    bool bc = inbounds != jl_false;
     BasicBlock *failBB = NULL, *endBB = NULL;
     if (bc) {
         failBB = BasicBlock::Create(ctx.builder.getContext(), "oob");
         endBB = BasicBlock::Create(ctx.builder.getContext(), "idxend");
     }
-#endif
     SmallVector<Value *> idxs(nidxs);
     for (size_t k = 0; k < nidxs; k++) {
         idxs[k] = emit_unbox(ctx, ctx.types().T_size, argv[k], (jl_value_t*)jl_long_type); // type asserted by caller
@@ -2979,7 +2960,6 @@ static Value *emit_array_nd_index(
         if (k < nidxs - 1) {
             assert(nd >= 0);
             Value *d = emit_arraysize_for_unsafe_dim(ctx, ainfo, ex, k + 1, nd);
-#if CHECK_BOUNDS==1
             if (bc) {
                 BasicBlock *okBB = BasicBlock::Create(ctx.builder.getContext(), "ib");
                 // if !(i < d) goto error
@@ -2989,12 +2969,10 @@ static Value *emit_array_nd_index(
                 ctx.f->getBasicBlockList().push_back(okBB);
                 ctx.builder.SetInsertPoint(okBB);
             }
-#endif
             stride = ctx.builder.CreateMul(stride, d);
             setName(ctx.emission_context, stride, "stride");
         }
     }
-#if CHECK_BOUNDS==1
     if (bc) {
         // We have already emitted a bounds check for each index except for
         // the last one which we therefore have to do here.
@@ -3055,7 +3033,6 @@ static Value *emit_array_nd_index(
         ctx.f->getBasicBlockList().push_back(endBB);
         ctx.builder.SetInsertPoint(endBB);
     }
-#endif
 
     return i;
 }