Skip to content

Commit

Permalink
Effects: Add :nothrow_if_inbounds effect
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Keno committed Mar 21, 2022
1 parent cf999af commit a517069
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 37 deletions.
38 changes: 29 additions & 9 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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{}
Expand Down Expand Up @@ -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
))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 20 additions & 2 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions base/compiler/ssair/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 15 additions & 3 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
33 changes: 23 additions & 10 deletions base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -47,31 +48,35 @@ 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)
return Effects(
consistent,
effect_free,
nothrow,
nothrow_if_inbounds,
terminates,
overlayed,
inbounds_taints_consistency)
Expand All @@ -86,25 +91,28 @@ 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(
TriState((e >> 0) & 0x03),
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

Expand All @@ -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,
Expand All @@ -126,6 +136,7 @@ struct EffectsOverride
consistent::Bool
effect_free::Bool
nothrow::Bool
nothrow_if_inbounds::Bool
terminates_globally::Bool
terminates_locally::Bool
end
Expand All @@ -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

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

"""
Expand Down
23 changes: 13 additions & 10 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit a517069

Please sign in to comment.