Skip to content

Commit

Permalink
effects: allow concrete-eval when --check-bounds=no if proven "safe" (
Browse files Browse the repository at this point in the history
#50107)

From version 1.9 onwards, when `--check-bounds=no` is used,
concrete-eval is completely disabled. However, it appears
`--check-bounds=no` is still being used within the community, causing
issues like the one reported in JuliaArrays/StaticArrays.jl#1155.
Although we should move forward to a direction of eliminating the flag
in the future (#48245), for the time being, there are many requests to
carry out a certain level of compiler optimization, even when this flag
is enabled.

This commit aims to allow concrete-eval "safely" even under
`--check-bounds=no`. Specifically, when the method call being analyzed
is `:nothrow`, it should be predominantly safe to concrete-eval it under
this flag. Technically, however, even `:nothrow` methods could trigger
undefined behavior, since `:nothrow` isn't a strict constraint and it's
possible for users to annotate potentially risky methods with
`Base.@assume_effects :nothrow`. Nonetheless, since this possibility is
acknowledged in `Base.@assume_effects` documentation, I feel it's fair
to relegate it to user responsibility.
  • Loading branch information
aviatesk authored Jun 12, 2023
1 parent bf9bbb2 commit 970941c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
20 changes: 13 additions & 7 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -834,21 +834,27 @@ end

function concrete_eval_eligible(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::AbsIntState)
(;effects) = result
if inbounds_option() === :off
# Disable concrete evaluation in `--check-bounds=no` mode, since we cannot be sure
# that inferred effects are accurate.
return :none
elseif !result.effects.noinbounds && stmt_taints_inbounds_consistency(sv)
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.
add_remark!(interp, sv, "[constprop] Concrete evel disabled for inbounds")
return :none
elseif isoverlayed(method_table(interp)) && !is_nonoverlayed(result.effects)
# disable all concrete-evaluation if this function call is tainted by some overlayed
end
if isoverlayed(method_table(interp)) && !is_nonoverlayed(effects)
# disable concrete-evaluation if this function call is tainted by some overlayed
# method since currently there is no direct way to execute overlayed methods
add_remark!(interp, sv, "[constprop] Concrete evel disabled for overlayed methods")
return :none
end
if result.edge !== nothing && is_foldable(result.effects)
if result.edge !== nothing && is_foldable(effects)
if f !== nothing && is_all_const_arg(arginfo, #=start=#2)
return :concrete_eval
elseif !any_conditional(arginfo)
Expand Down
6 changes: 5 additions & 1 deletion test/boundscheck_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ begin # Pass inbounds meta to getindex on CartesianIndices (#42115)
end
end


# Test that --check-bounds=off doesn't permit const prop of indices into
# function that are not dynamically reachable (the same test for @inbounds
# is in the compiler tests).
Expand All @@ -294,4 +293,9 @@ function f_boundscheck_elim(n)
end
@test Tuple{} <: code_typed(f_boundscheck_elim, Tuple{Int})[1][2]

# https://github.com/JuliaArrays/StaticArrays.jl/issues/1155
@test Base.return_types() do
typeintersect(Int, Integer)
end |> only === Type{Int}

end

0 comments on commit 970941c

Please sign in to comment.