From a51706919bce8bf0182eefd4d6317c3eefb8a6a5 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 21 Mar 2022 22:02:17 +0000 Subject: [PATCH] Effects: Add :nothrow_if_inbounds effect This adds a new effect :nothrow_if_inbounds that basically does what it says on the tin. Running #43852 through its paces, I've found it highly effective, except in cases where a function was only nothrow because of a boundscheck with unknown bounds. This PR adds support to make use of @inbounds annotations to still let inlining remove calls that are nothrow except for an unknown boundscheck. --- base/compiler/abstractinterpretation.jl | 38 +++++++++++++++++++------ base/compiler/inferencestate.jl | 2 +- base/compiler/ssair/inlining.jl | 22 ++++++++++++-- base/compiler/ssair/show.jl | 2 ++ base/compiler/tfuncs.jl | 18 ++++++++++-- base/compiler/types.jl | 33 ++++++++++++++------- src/julia.h | 23 ++++++++------- test/compiler/inline.jl | 8 ++++++ test/compiler/irpasses.jl | 4 +-- 9 files changed, 113 insertions(+), 37 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index cf6af9ce17acb..fb01eb1602c63 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -36,6 +36,22 @@ function should_infer_for_effects(sv::InferenceState) sv.ipo_effects.effect_free === ALWAYS_TRUE end +function merge_statement_effect!(sv::InferenceState, effects::Effects) + stmt_inbounds = get_curr_ssaflag(sv) & IR_FLAG_INBOUNDS != 0 + propagate_inbounds = sv.src.propagate_inbounds + # Look at inbounds state and see what we need to do about :nothrow_if_inbounds + adjusted_effects = Effects( + effects.consistent, + effects.effect_free, + stmt_inbounds ? effects.nothrow_if_inbounds : effects.nothrow, + propagate_inbounds ? effects.nothrow_if_inbounds : + (effects.nothrow === ALWAYS_TRUE ? ALWAYS_TRUE : TRISTATE_UNKNOWN), + effects.terminates, + effects.overlayed + ) + tristate_merge!(sv, adjusted_effects) +end + function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), arginfo::ArgInfo, @nospecialize(atype), sv::InferenceState, max_methods::Int) @@ -133,7 +149,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), (; rt, effects, const_result) = const_call_result end end - tristate_merge!(sv, effects) + merge_statement_effect!(sv, effects) push!(const_results, const_result) if const_result !== nothing any_const_result = true @@ -180,7 +196,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), (; effects, const_result) = const_call_result end end - tristate_merge!(sv, effects) + merge_statement_effect!(sv, effects) push!(const_results, const_result) if const_result !== nothing any_const_result = true @@ -217,7 +233,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), elseif isa(matches, MethodMatches) ? (!matches.fullmatch || any_ambig(matches)) : (!_all(b->b, matches.fullmatches) || any_ambig(matches)) # Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature. - tristate_merge!(sv, Effects(EFFECTS_TOTAL; nothrow=TRISTATE_UNKNOWN)) + tristate_merge!(sv, Effects(EFFECTS_TOTAL, nothrow=TRISTATE_UNKNOWN, nothrow_if_inbounds=TRISTATE_UNKNOWN)) end rettype = from_interprocedural!(rettype, sv, arginfo, conditionals) @@ -1574,7 +1590,7 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f), return abstract_modifyfield!(interp, argtypes, sv) end rt = abstract_call_builtin(interp, f, arginfo, sv, max_methods) - tristate_merge!(sv, builtin_effects(f, argtypes, rt)) + tristate_merge!(sv, builtin_effects(f, arginfo, rt)) return CallMeta(rt, false) elseif isa(f, Core.OpaqueClosure) # calling an OpaqueClosure about which we have no information returns no information @@ -1902,7 +1918,8 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), end tristate_merge!(sv, Effects(EFFECTS_TOTAL; consistent = !ismutabletype(t) ? ALWAYS_TRUE : ALWAYS_FALSE, - nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE)) + nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE, + nothrow_if_inbounds = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE)) elseif ehead === :splatnew t, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], vtypes, sv)) is_nothrow = false # TODO: More precision @@ -1921,7 +1938,8 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), end tristate_merge!(sv, Effects(EFFECTS_TOTAL; consistent = ismutabletype(t) ? ALWAYS_FALSE : ALWAYS_TRUE, - nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE)) + nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE, + nothrow_if_inbounds = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE)) elseif ehead === :new_opaque_closure tristate_merge!(sv, Effects()) # TODO t = Union{} @@ -1960,6 +1978,7 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), effects.consistent ? ALWAYS_TRUE : TRISTATE_UNKNOWN, effects.effect_free ? ALWAYS_TRUE : TRISTATE_UNKNOWN, effects.nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN, + effects.nothrow_if_inbounds ? ALWAYS_TRUE : TRISTATE_UNKNOWN, effects.terminates_globally ? ALWAYS_TRUE : TRISTATE_UNKNOWN, #=overlayed=#false )) @@ -2045,7 +2064,7 @@ function abstract_eval_global(M::Module, s::Symbol, frame::InferenceState) if isdefined(M,s) tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE)) else - tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE, nothrow=ALWAYS_FALSE)) + tristate_merge!(frame, Effects(EFFECTS_TOTAL, consistent=ALWAYS_FALSE, nothrow=ALWAYS_FALSE, nothrow_if_inbounds=ALWAYS_FALSE)) end return ty end @@ -2285,8 +2304,9 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState) changes = StateUpdate(lhs, VarState(t, false), changes, false) elseif isa(lhs, GlobalRef) tristate_merge!(frame, Effects(EFFECTS_TOTAL, - effect_free=ALWAYS_FALSE, - nothrow=TRISTATE_UNKNOWN)) + effect_free=ALWAYS_FALSE, + nothrow=TRISTATE_UNKNOWN, + nothrow_if_inbounds=TRISTATE_UNKNOWN)) elseif !isa(lhs, SSAValue) tristate_merge!(frame, Effects(; overlayed=false)) end diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl index 7d2f69cbab311..59313b701d0db 100644 --- a/base/compiler/inferencestate.jl +++ b/base/compiler/inferencestate.jl @@ -198,7 +198,7 @@ mutable struct InferenceState #=parent=#nothing, #=cached=#cache === :global, #=inferred=#false, #=dont_work_on_me=#false, #=restrict_abstract_call_sites=# isa(linfo.def, Module), - #=ipo_effects=#Effects(consistent, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false, inbounds_taints_consistency), + #=ipo_effects=#Effects(consistent, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false, inbounds_taints_consistency), interp) result.result = frame cache !== :no && push!(get_inference_cache(interp), result) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 225bd46c6f262..5cfc5f4441a41 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -513,7 +513,7 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int, if isa(case, InliningTodo) val = ir_inline_item!(compact, idx, argexprs′, linetable, case, boundscheck, todo_bbs) elseif isa(case, InvokeCase) - effect_free = is_removable_if_unused(case.effects) + effect_free = is_removable_if_unused(case.effects, boundscheck === :off) val = insert_node_here!(compact, NewInstruction(Expr(:invoke, case.invoke, argexprs′...), typ, nothing, line, effect_free ? IR_FLAG_EFFECT_FREE : IR_FLAG_NULL, effect_free)) @@ -877,7 +877,7 @@ function handle_single_case!( isinvoke && rewrite_invoke_exprargs!(stmt) stmt.head = :invoke pushfirst!(stmt.args, case.invoke) - if is_removable_if_unused(case.effects) + if is_removable_if_unused(case.effects, ir[SSAValue(idx)][:flag] & IR_FLAG_INBOUNDS != 0) ir[SSAValue(idx)][:flag] |= IR_FLAG_EFFECT_FREE end elseif case === nothing @@ -1345,6 +1345,24 @@ function handle_cases!(ir::IRCode, idx::Int, stmt::Expr, @nospecialize(atype), if fully_covered && length(cases) == 1 handle_single_case!(ir, idx, stmt, cases[1].item, todo, params) elseif length(cases) > 0 + all_removable = true + for case in cases + if isa(case, InvokeCase) + effects = case.effects + elseif isa(case, InliningTodo) + effects = case.spec.effects + else + continue + end + if !is_total(case.effects) && + !(is_removable_if_unused(case.effects, ir[SSAValue(idx)][:flag] & IR_FLAG_INBOUNDS != 0)) + all_removable = false + break + end + end + if all_removable + ir[SSAValue(idx)][:flag] |= IR_FLAG_EFFECT_FREE + end isa(atype, DataType) || return nothing all(case::InliningCase->isa(case.sig, DataType), cases) || return nothing push!(todo, idx=>UnionSplit(fully_covered, atype, cases)) diff --git a/base/compiler/ssair/show.jl b/base/compiler/ssair/show.jl index 76cbcbd4d5d7d..6e528a91c0a87 100644 --- a/base/compiler/ssair/show.jl +++ b/base/compiler/ssair/show.jl @@ -801,6 +801,8 @@ function Base.show(io::IO, e::Core.Compiler.Effects) print(io, ',') printstyled(io, string(tristate_letter(e.nothrow), 'n'); color=tristate_color(e.nothrow)) print(io, ',') + printstyled(io, string(tristate_letter(e.nothrow_if_inbounds), 'i'); color=tristate_color(e.nothrow_if_inbounds)) + print(io, ',') printstyled(io, string(tristate_letter(e.terminates), 't'); color=tristate_color(e.terminates)) print(io, ')') e.overlayed && printstyled(io, '′'; color=:red) diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index b204556bed83d..0b9855e1a2120 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -1643,7 +1643,7 @@ function array_type_undefable(@nospecialize(arytype)) end function array_builtin_common_nothrow(argtypes::Vector{Any}, first_idx_idx::Int) - length(argtypes) >= 4 || return false + length(argtypes) >= first_idx_idx || return false boundscheck = argtypes[1] arytype = argtypes[2] array_builtin_common_typecheck(boundscheck, arytype, argtypes, first_idx_idx) || return false @@ -1778,10 +1778,11 @@ const _SPECIAL_BUILTINS = Any[ Core._apply_iterate ] -function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt) +function builtin_effects(f::Builtin, arginfo::ArgInfo, rt) if isa(f, IntrinsicFunction) - return intrinsic_effects(f, argtypes) + return intrinsic_effects(f, arginfo.argtypes) end + (;argtypes, fargs) = arginfo @assert !contains_is(_SPECIAL_BUILTINS, f) @@ -1822,10 +1823,20 @@ function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt) nothrow = isvarargtype(argtypes[end]) ? false : builtin_nothrow(f, argtypes[2:end], rt) end + nothrow_if_inbounds = nothrow + + if !nothrow && f === Core.arrayref && fargs !== nothing && length(fargs) >= 3 && + isexpr(fargs[2], :boundscheck) && !isvarargtype(argtypes[end]) + new_argtypes = argtypes[3:end] + pushfirst!(new_argtypes, Const(false)) + nothrow_if_inbounds = builtin_nothrow(f, new_argtypes, rt) + end + return Effects( ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE, effect_free ? ALWAYS_TRUE : ALWAYS_FALSE, nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN, + nothrow_if_inbounds ? ALWAYS_TRUE : TRISTATE_UNKNOWN, #=terminates=#ALWAYS_TRUE, #=overlayed=#false, ) @@ -2009,6 +2020,7 @@ function intrinsic_effects(f::IntrinsicFunction, argtypes::Vector{Any}) ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE, effect_free ? ALWAYS_TRUE : ALWAYS_FALSE, nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN, + nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN, #=terminates=#ALWAYS_TRUE, #=overlayed=#false, ) diff --git a/base/compiler/types.jl b/base/compiler/types.jl index eff601dd7dc6e..1d49d3754747a 100644 --- a/base/compiler/types.jl +++ b/base/compiler/types.jl @@ -37,6 +37,7 @@ struct Effects consistent::TriState effect_free::TriState nothrow::TriState + nothrow_if_inbounds::TriState terminates::TriState overlayed::Bool # This effect is currently only tracked in inference and modified @@ -47,24 +48,27 @@ function Effects( consistent::TriState, effect_free::TriState, nothrow::TriState, + nothrow_if_inbounds::TriState, terminates::TriState, overlayed::Bool) return Effects( consistent, effect_free, nothrow, + nothrow_if_inbounds, terminates, overlayed, false) end -const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false) -const EFFECTS_UNKNOWN = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, true) +const EFFECTS_TOTAL = Effects(ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE, false) +const EFFECTS_UNKNOWN = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, true) function Effects(e::Effects = EFFECTS_UNKNOWN; consistent::TriState = e.consistent, effect_free::TriState = e.effect_free, nothrow::TriState = e.nothrow, + nothrow_if_inbounds::TriState=e.nothrow_if_inbounds, terminates::TriState = e.terminates, overlayed::Bool = e.overlayed, inbounds_taints_consistency::Bool = e.inbounds_taints_consistency) @@ -72,6 +76,7 @@ function Effects(e::Effects = EFFECTS_UNKNOWN; consistent, effect_free, nothrow, + nothrow_if_inbounds, terminates, overlayed, inbounds_taints_consistency) @@ -86,17 +91,19 @@ is_total(effects::Effects) = is_total_or_error(effects) && effects.nothrow === ALWAYS_TRUE -is_removable_if_unused(effects::Effects) = +is_removable_if_unused(effects::Effects, assume_inbounds::Bool) = effects.effect_free === ALWAYS_TRUE && effects.terminates === ALWAYS_TRUE && - effects.nothrow === ALWAYS_TRUE + (effects.nothrow === ALWAYS_TRUE || + (assume_inbounds && effects.nothrow_if_inbounds === ALWAYS_TRUE)) function encode_effects(e::Effects) return (e.consistent.state << 0) | (e.effect_free.state << 2) | (e.nothrow.state << 4) | - (e.terminates.state << 6) | - (UInt32(e.overlayed) << 8) + (e.nothrow_if_inbounds.state << 6) | + (UInt32(e.terminates.state) << 8) | + (UInt32(e.overlayed) << 10) end function decode_effects(e::UInt32) return Effects( @@ -104,7 +111,8 @@ function decode_effects(e::UInt32) TriState((e >> 2) & 0x03), TriState((e >> 4) & 0x03), TriState((e >> 6) & 0x03), - _Bool( (e >> 8) & 0x01), + TriState((e >> 8) & 0x03), + _Bool( (e >> 10) & 0x01), false) end @@ -116,6 +124,8 @@ function tristate_merge(old::Effects, new::Effects) old.effect_free, new.effect_free), tristate_merge( old.nothrow, new.nothrow), + tristate_merge( + old.nothrow_if_inbounds, new.nothrow_if_inbounds), tristate_merge( old.terminates, new.terminates), old.overlayed | new.overlayed, @@ -126,6 +136,7 @@ struct EffectsOverride consistent::Bool effect_free::Bool nothrow::Bool + nothrow_if_inbounds::Bool terminates_globally::Bool terminates_locally::Bool end @@ -135,8 +146,9 @@ function encode_effects_override(eo::EffectsOverride) eo.consistent && (e |= 0x01) eo.effect_free && (e |= 0x02) eo.nothrow && (e |= 0x04) - eo.terminates_globally && (e |= 0x08) - eo.terminates_locally && (e |= 0x10) + eo.nothrow_if_inbounds && (e |= 0x08) + eo.terminates_globally && (e |= 0x10) + eo.terminates_locally && (e |= 0x20) return e end @@ -146,7 +158,8 @@ function decode_effects_override(e::UInt8) (e & 0x02) != 0x00, (e & 0x04) != 0x00, (e & 0x08) != 0x00, - (e & 0x10) != 0x00) + (e & 0x10) != 0x00, + (e & 0x20) != 0x00) end """ diff --git a/src/julia.h b/src/julia.h index 3153b87c3a9b9..cdf1092f06022 100644 --- a/src/julia.h +++ b/src/julia.h @@ -238,6 +238,7 @@ typedef union __jl_purity_overrides_t { uint8_t ipo_consistent : 1; uint8_t ipo_effect_free : 1; uint8_t ipo_nothrow : 1; + uint8_t ipo_nothrow_if_inbounds : 1; uint8_t ipo_terminates : 1; // Weaker form of `terminates` that asserts // that any control flow syntactically in the method @@ -396,21 +397,23 @@ typedef struct _jl_code_instance_t { union { uint32_t ipo_purity_bits; struct { - uint8_t ipo_consistent:2; - uint8_t ipo_effect_free:2; - uint8_t ipo_nothrow:2; - uint8_t ipo_terminates:2; - uint8_t ipo_overlayed:1; + uint32_t ipo_consistent:2; + uint32_t ipo_effect_free:2; + uint32_t ipo_nothrow:2; + uint32_t ipo_terminates:2; + uint32_t ipo_nothrow_if_inbounds:2; + uint32_t ipo_overlayed:1; } ipo_purity_flags; }; union { uint32_t purity_bits; struct { - uint8_t consistent:2; - uint8_t effect_free:2; - uint8_t nothrow:2; - uint8_t terminates:2; - uint8_t overlayed:1; + uint32_t consistent:2; + uint32_t effect_free:2; + uint32_t nothrow:2; + uint32_t terminates:2; + uint32_t ipo_nothrow_if_inbounds:2; + uint32_t overlayed:1; } purity_flags; }; jl_value_t *argescapes; // escape information of call arguments diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index fa4425893767c..952c6272064a3 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -1046,6 +1046,14 @@ function call_call_ambig(b::Bool) end @test !fully_eliminated(call_call_ambig, Tuple{Bool}) +# unusued, total, noinline, propagates_inbounds to arrayref +@noinline Base.@propagate_inbounds f_total_noinline_propagates_inbounds(x, i) = x[i] +function f_call_total_noinline_propgates_inbounds(x) + @inbounds f_total_noinline_propagates_inbounds(x, 1) + return nothing +end +@test fully_eliminated(f_call_total_noinline_propgates_inbounds, Tuple{Vector{Float64}}) + # Test that a missing methtable identification gets tainted # appropriately struct FCallback; f::Union{Nothing, Function}; end diff --git a/test/compiler/irpasses.jl b/test/compiler/irpasses.jl index 045cf833944c2..25de4fe72a04d 100644 --- a/test/compiler/irpasses.jl +++ b/test/compiler/irpasses.jl @@ -832,8 +832,8 @@ let ci = code_typed(foo_cfg_empty, Tuple{Bool}, optimize=true)[1][1] @test isa(ir.stmts[length(ir.stmts)][:inst], ReturnNode) end -@test Core.Compiler.builtin_effects(getfield, Any[Complex{Int}, Symbol], Any).effect_free.state == 0x01 -@test Core.Compiler.builtin_effects(getglobal, Any[Module, Symbol], Any).effect_free.state == 0x01 +@test Core.Compiler.builtin_effects(getfield, Core.Compiler.ArgInfo(nothing, Any[Complex{Int}, Symbol]), Any).effect_free.state == 0x01 +@test Core.Compiler.builtin_effects(getglobal, Core.Compiler.ArgInfo(nothing, Any[Module, Symbol]), Any).effect_free.state == 0x01 # Test that UseRefIterator gets SROA'd inside of new_to_regular (#44557) # expression and new_to_regular offset are arbitrary here, we just want to see the UseRefIterator erased let e = Expr(:call, Core.GlobalRef(Base, :arrayset), false, Core.SSAValue(4), Core.SSAValue(9), Core.SSAValue(8))