diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index a35383ea4e6b9..9dcc48841c24f 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -781,9 +781,18 @@ end # - false: eligible for semi-concrete evaluation # - nothing: not eligible for either of it function concrete_eval_eligible(interp::AbstractInterpreter, - @nospecialize(f), result::MethodCallResult, arginfo::ArgInfo) + @nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState) # disable all concrete-evaluation if this function call is tainted by some overlayed # method since currently there is no direct way to execute overlayed methods + if inbounds_option() === :off + # Disable concrete evaluation in `--check-bounds=no` mode, since we cannot be sure + # that inferred effects are accurate. + return nothing + elseif !result.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. + return nothing + end isoverlayed(method_table(interp)) && !is_nonoverlayed(result.effects) && return nothing if f !== nothing && result.edge !== nothing && is_foldable(result.effects) if is_all_const_arg(arginfo, #=start=#2) @@ -824,7 +833,7 @@ end function concrete_eval_call(interp::AbstractInterpreter, @nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, si::StmtInfo, sv::InferenceState, invokecall::Union{Nothing,InvokeCall}=nothing) - eligible = concrete_eval_eligible(interp, f, result, arginfo) + eligible = concrete_eval_eligible(interp, f, result, arginfo, sv) eligible === nothing && return false if eligible args = collect_const_args(arginfo, #=start=#2) @@ -2036,21 +2045,18 @@ function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, vtypes:: end elseif head === :boundscheck if isa(sv, InferenceState) - stmt = sv.src.code[sv.currpc] - if isexpr(stmt, :call) - f = abstract_eval_value(interp, stmt.args[1], vtypes, sv) - if f isa Const && f.val === getfield - # boundscheck of `getfield` call is analyzed by tfunc potentially without - # tainting :consistent-cy when it's known to be nothrow - @goto delay_effects_analysis - end - end + flag = sv.src.ssaflags[sv.currpc] + # If there is no particular @inbounds for this function, then we only taint `noinbounds`, + # which will subsequently taint consistency if this function is called from another + # function that uses `@inbounds`. However, if this :boundscheck is itself within an + # `@inbounds` region, its value depends on `--check-bounds`, so we need to taint + # consistency here also. + merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; noinbounds=false, + consistent = (flag & IR_FLAG_INBOUNDS) != 0 ? ALWAYS_FALSE : ALWAYS_TRUE)) end - merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE, noinbounds=false)) - @label delay_effects_analysis rt = Bool elseif head === :inbounds - merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE, noinbounds=false)) + @assert false && "Expected this to have been moved into flags" elseif head === :the_exception merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE)) end @@ -2125,7 +2131,6 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp arginfo = ArgInfo(ea, argtypes) si = StmtInfo(isa(sv, IRCode) ? true : !call_result_unused(sv, sv.currpc)) (; rt, effects, info) = abstract_call(interp, arginfo, si, sv) - merge_effects!(interp, sv, effects) if isa(sv, InferenceState) sv.stmt_info[sv.currpc] = info end @@ -2189,7 +2194,6 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp nothrow = false end effects = Effects(EFFECTS_TOTAL; consistent, nothrow) - merge_effects!(interp, sv, effects) elseif ehead === :splatnew t, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], vtypes, sv)) nothrow = false # TODO: More precision @@ -2208,7 +2212,6 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp end consistent = !ismutabletype(t) ? ALWAYS_TRUE : CONSISTENT_IF_NOTRETURNED effects = Effects(EFFECTS_TOTAL; consistent, nothrow) - merge_effects!(interp, sv, effects) elseif ehead === :new_opaque_closure t = Union{} effects = Effects() # TODO @@ -2236,20 +2239,16 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp elseif ehead === :foreigncall (;rt, effects) = abstract_eval_foreigncall(interp, e, vtypes, sv, mi) t = rt - merge_effects!(interp, sv, effects) elseif ehead === :cfunction effects = EFFECTS_UNKNOWN - merge_effects!(interp, sv, effects) t = e.args[1] isa(t, Type) || (t = Any) abstract_eval_cfunction(interp, e, vtypes, sv) elseif ehead === :method t = (length(e.args) == 1) ? Any : Nothing effects = EFFECTS_UNKNOWN - merge_effects!(interp, sv, effects) elseif ehead === :copyast effects = EFFECTS_UNKNOWN - merge_effects!(interp, sv, effects) t = abstract_eval_value(interp, e.args[1], vtypes, sv) if t isa Const && t.val isa Expr # `copyast` makes copies of Exprs @@ -2260,6 +2259,7 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp elseif ehead === :isdefined sym = e.args[1] t = Bool + effects = EFFECTS_TOTAL if isa(sym, SlotNumber) vtyp = vtypes[slot_id(sym)] if vtyp.typ === Bottom @@ -2288,9 +2288,9 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp @label always_throw t = Bottom effects = EFFECTS_THROWS - merge_effects!(interp, sv, effects) else t = abstract_eval_value_expr(interp, e, vtypes, sv) + effects = EFFECTS_TOTAL end return RTEffects(t, effects) end @@ -2332,6 +2332,11 @@ function abstract_eval_phi(interp::AbstractInterpreter, phi::PhiNode, vtypes::Un return rt end +function stmt_taints_inbounds_consistency(sv::InferenceState) + flag = sv.src.ssaflags[sv.currpc] + return sv.src.propagate_inbounds || (flag & IR_FLAG_INBOUNDS) != 0 +end + function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), vtypes::VarTable, sv::InferenceState) if !isa(e, Expr) if isa(e, PhiNode) @@ -2340,6 +2345,18 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), return abstract_eval_special_value(interp, e, vtypes, sv) end (;rt, effects) = abstract_eval_statement_expr(interp, e, vtypes, sv, nothing) + if !effects.noinbounds + flag = sv.src.ssaflags[sv.currpc] + if !sv.src.propagate_inbounds + # The callee read our inbounds flag, but unless we propagate inbounds, + # we ourselves don't read our parent's inbounds. + effects = Effects(effects; noinbounds=true) + end + if (flag & IR_FLAG_INBOUNDS) != 0 + effects = Effects(effects; consistent=ALWAYS_FALSE) + end + end + merge_effects!(interp, sv, effects) e = e::Expr @assert !isa(rt, TypeVar) "unhandled TypeVar" rt = maybe_singleton_const(rt) diff --git a/base/compiler/effects.jl b/base/compiler/effects.jl index b935f2ab81385..48842d7f51cd8 100644 --- a/base/compiler/effects.jl +++ b/base/compiler/effects.jl @@ -40,9 +40,9 @@ following meanings: This state corresponds to LLVM's `inaccessiblemem_or_argmemonly` function attribute. - `nonoverlayed::Bool`: indicates that any methods that may be called within this method are not defined in an [overlayed method table](@ref OverlayMethodTable). -- `noinbounds::Bool`: indicates this method can't be `:consistent` because of bounds checking. - This effect is currently only set on `InferenceState` construction and used to taint - `:consistent`-cy before caching. We may want to track it with more accuracy in the future. +- `noinbounds::Bool`: If set, indicates that this method does not read the parent's :inbounds + state. In particular, it does not have any reached :boundscheck exprs, not propagates inbounds + to any children that do. Note that the representations above are just internal implementation details and thus likely to change in the future. See [`Base.@assume_effects`](@ref) for more detailed explanation @@ -98,10 +98,10 @@ const EFFECT_FREE_IF_INACCESSIBLEMEMONLY = 0x01 << 1 # :inaccessiblememonly bits const INACCESSIBLEMEM_OR_ARGMEMONLY = 0x01 << 1 -const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, true, true, true, ALWAYS_TRUE, true) -const EFFECTS_THROWS = Effects(ALWAYS_TRUE, ALWAYS_TRUE, false, true, true, ALWAYS_TRUE, true) -const EFFECTS_UNKNOWN = Effects(ALWAYS_FALSE, ALWAYS_FALSE, false, false, false, ALWAYS_FALSE, true) # unknown mostly, but it's not overlayed at least (e.g. it's not a call) -const EFFECTS_UNKNOWN′ = Effects(ALWAYS_FALSE, ALWAYS_FALSE, false, false, false, ALWAYS_FALSE, false) # unknown really +const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, true, true, true, ALWAYS_TRUE, true, true) +const EFFECTS_THROWS = Effects(ALWAYS_TRUE, ALWAYS_TRUE, false, true, true, ALWAYS_TRUE, true, true) +const EFFECTS_UNKNOWN = Effects(ALWAYS_FALSE, ALWAYS_FALSE, false, false, false, ALWAYS_FALSE, true, true) # unknown mostly, but it's not overlayed at least (e.g. it's not a call) +const EFFECTS_UNKNOWN′ = Effects(ALWAYS_FALSE, ALWAYS_FALSE, false, false, false, ALWAYS_FALSE, false, true) # unknown really function Effects(e::Effects = EFFECTS_UNKNOWN′; consistent::UInt8 = e.consistent, @@ -184,7 +184,8 @@ function encode_effects(e::Effects) ((e.terminates % UInt32) << 6) | ((e.notaskstate % UInt32) << 7) | ((e.inaccessiblememonly % UInt32) << 8) | - ((e.nonoverlayed % UInt32) << 10) + ((e.nonoverlayed % UInt32) << 10)| + ((e.noinbounds % UInt32) << 11) end function decode_effects(e::UInt32) @@ -195,7 +196,8 @@ function decode_effects(e::UInt32) _Bool((e >> 6) & 0x01), _Bool((e >> 7) & 0x01), UInt8((e >> 8) & 0x03), - _Bool((e >> 10) & 0x01)) + _Bool((e >> 10) & 0x01), + _Bool((e >> 11) & 0x01)) end struct EffectsOverride diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl index a02fc9fc552fc..cb6ebe5e76dc3 100644 --- a/base/compiler/inferencestate.jl +++ b/base/compiler/inferencestate.jl @@ -177,16 +177,7 @@ mutable struct InferenceState valid_worlds = WorldRange(src.min_world, src.max_world == typemax(UInt) ? get_world_counter() : src.max_world) bestguess = Bottom - # TODO: Currently, any :inbounds declaration taints consistency, - # because we cannot be guaranteed whether or not boundschecks - # will be eliminated and if they are, we cannot be guaranteed - # that no undefined behavior will occur (the effects assumptions - # are stronger than the inbounds assumptions, since the latter - # requires dynamic reachability, while the former is global). - inbounds = inbounds_option() - noinbounds = inbounds === :on || (inbounds === :default && all(flag::UInt8->iszero(flag&IR_FLAG_INBOUNDS), src.ssaflags)) - consistent = noinbounds ? ALWAYS_TRUE : ALWAYS_FALSE - ipo_effects = Effects(EFFECTS_TOTAL; consistent, noinbounds) + ipo_effects = Effects(EFFECTS_TOTAL) params = InferenceParams(interp) restrict_abstract_call_sites = isa(linfo.def, Module) diff --git a/base/compiler/ssair/show.jl b/base/compiler/ssair/show.jl index d2291404f11c6..f4d240f423e89 100644 --- a/base/compiler/ssair/show.jl +++ b/base/compiler/ssair/show.jl @@ -1012,6 +1012,8 @@ function Base.show(io::IO, e::Effects) printstyled(io, effectbits_letter(e, :notaskstate, 's'); color=effectbits_color(e, :notaskstate)) print(io, ',') printstyled(io, effectbits_letter(e, :inaccessiblememonly, 'm'); color=effectbits_color(e, :inaccessiblememonly)) + print(io, ',') + printstyled(io, effectbits_letter(e, :noinbounds, 'i'); color=effectbits_color(e, :noinbounds)) print(io, ')') e.nonoverlayed || printstyled(io, '′'; color=:red) end diff --git a/base/expr.jl b/base/expr.jl index 376ed43ba52f0..76e9c3af9a2c6 100644 --- a/base/expr.jl +++ b/base/expr.jl @@ -607,6 +607,10 @@ currently equivalent to the following `setting`s: however, that by the `:consistent`-cy requirements, any such annotated call must consistently throw given the same argument values. +!!! note + An explict `@inbounds` annotation inside the function will also disable + constant propagation and not be overriden by :foldable. + --- ## `:total` diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index 3680a0c9a44cb..5e18d5386089d 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -680,3 +680,13 @@ mksparamunused(x) = (SparamUnused(x); nothing) let src = code_typed1(mksparamunused, (Any,)) @test count(isnew, src.code) == 0 end + +# Test that dead `@inbounds` does not taint consistency +@test Base.infer_effects() do + false && @inbounds (1,2,3)[1] + return 1 +end |> Core.Compiler.is_total + +@test Base.infer_effects(Tuple{Int64}) do i + @inbounds (1,2,3)[i] +end |> !Core.Compiler.is_consistent