Skip to content

Commit

Permalink
Add an option to not emit unionsplit fallback blocks
Browse files Browse the repository at this point in the history 
When union splitting, we currently emit a fallback block that prints
`fatal error in type inference (type bound)`. This has always been an oddity,
because there are plenty of other places in the compiler that we
rely on the correctness of inference, but it has caught a number of
issues over the years and we do still have a few issues (e.g. #43064)
that show this error. Nevertheless, the occurrence of issues like
this has become less frequent, so it might be time to turn it off soon.
At the same time, we have downstream users of the compiler infrastructure
that get confused by having extra `throw` calls inserted into functions
that (post-#43852) were inferred as `:nothrow`.

Here we add an optimization param (defaulted to on) to determine whether
or not to insert the unionsplit fallback block. Because of the conservative
default, we can decide later what the correct default is (maybe turn it on
in `debug` mode?), while letting downstream consumers play with the setting
for now to see if any issues crop up.
  • Loading branch information
@Keno
Keno committed Feb 8, 2022
1 parent d7fd3b7 commit 8b8e338
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 35 deletions.
75 changes: 41 additions & 34 deletions base/compiler/ssair/inlining.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function ssa_inlining_pass!(ir::IRCode, linetable::Vector{LineInfoNode}, state::
@timeit "analysis" todo = assemble_inline_todo!(ir, state)
isempty(todo) && return ir
# Do the actual inlining for every call we identified
@timeit "execution" ir = batch_inline!(todo, ir, linetable, propagate_inbounds)
@timeit "execution" ir = batch_inline!(todo, ir, linetable, propagate_inbounds, state.params)
return ir
end

Expand Down Expand Up @@ -210,7 +210,8 @@ end

function cfg_inline_unionsplit!(ir::IRCode, idx::Int,
(; fully_covered, #=atype,=# cases, bbs)::UnionSplit,
state::CFGInliningState)
state::CFGInliningState,
params::OptimizationParams)
inline_into_block!(state, block_for_inst(ir, idx))
from_bbs = Int[]
delete!(state.split_targets, length(state.new_cfg_blocks))
Expand All @@ -233,7 +234,7 @@ function cfg_inline_unionsplit!(ir::IRCode, idx::Int,
push!(from_bbs, length(state.new_cfg_blocks))
# TODO: Right now we unconditionally generate a fallback block
# in case of subtyping errors - This is probably unnecessary.
if true # i != length(cases) || !fully_covered
if i != length(cases) || (!fully_covered || !params.trust_inference)
# This block will have the next condition or the final else case
push!(state.new_cfg_blocks, BasicBlock(StmtRange(idx, idx)))
push!(state.new_cfg_blocks[cond_bb].succs, length(state.new_cfg_blocks))
Expand Down Expand Up @@ -457,12 +458,13 @@ const FATAL_TYPE_BOUND_ERROR = ErrorException("fatal error in type inference (ty
function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
argexprs::Vector{Any}, linetable::Vector{LineInfoNode},
(; fully_covered, atype, cases, bbs)::UnionSplit,
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
params::OptimizationParams)
stmt, typ, line = compact.result[idx][:inst], compact.result[idx][:type], compact.result[idx][:line]
join_bb = bbs[end]
pn = PhiNode()
local bb = compact.active_result_bb
@assert length(bbs) > length(cases)
@assert length(bbs) >= length(cases)
for i in 1:length(cases)
ithcase = cases[i]
metharg = ithcase.sig
Expand All @@ -472,21 +474,23 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
cond = true
aparams, mparams = atype.parameters::SimpleVector, metharg.parameters::SimpleVector
@assert length(aparams) == length(mparams)
for i in 1:length(aparams)
a, m = aparams[i], mparams[i]
# If this is always true, we don't need to check for it
a <: m && continue
# Generate isa check
isa_expr = Expr(:call, isa, argexprs[i], m)
ssa = insert_node_here!(compact, NewInstruction(isa_expr, Bool, line))
if cond === true
cond = ssa
else
and_expr = Expr(:call, and_int, cond, ssa)
cond = insert_node_here!(compact, NewInstruction(and_expr, Bool, line))
if i != length(cases) || !fully_covered || !params.trust_inference
for i in 1:length(aparams)
a, m = aparams[i], mparams[i]
# If this is always true, we don't need to check for it
a <: m && continue
# Generate isa check
isa_expr = Expr(:call, isa, argexprs[i], m)
ssa = insert_node_here!(compact, NewInstruction(isa_expr, Bool, line))
if cond === true
cond = ssa
else
and_expr = Expr(:call, and_int, cond, ssa)
cond = insert_node_here!(compact, NewInstruction(and_expr, Bool, line))
end
end
insert_node_here!(compact, NewInstruction(GotoIfNot(cond, next_cond_bb), Union{}, line))
end
insert_node_here!(compact, NewInstruction(GotoIfNot(cond, next_cond_bb), Union{}, line))
bb = next_cond_bb - 1
finish_current_bb!(compact, 0)
argexprs′ = argexprs
Expand Down Expand Up @@ -525,10 +529,12 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
bb += 1
# We're now in the fall through block, decide what to do
if fully_covered
e = Expr(:call, GlobalRef(Core, :throw), FATAL_TYPE_BOUND_ERROR)
insert_node_here!(compact, NewInstruction(e, Union{}, line))
insert_node_here!(compact, NewInstruction(ReturnNode(), Union{}, line))
finish_current_bb!(compact, 0)
if !params.trust_inference
e = Expr(:call, GlobalRef(Core, :throw), FATAL_TYPE_BOUND_ERROR)
insert_node_here!(compact, NewInstruction(e, Union{}, line))
insert_node_here!(compact, NewInstruction(ReturnNode(), Union{}, line))
finish_current_bb!(compact, 0)
end
else
ssa = insert_node_here!(compact, NewInstruction(stmt, typ, line))
push!(pn.edges, bb)
Expand All @@ -541,12 +547,12 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
return insert_node_here!(compact, NewInstruction(pn, typ, line))
end

function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vector{LineInfoNode}, propagate_inbounds::Bool)
function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vector{LineInfoNode}, propagate_inbounds::Bool, params::OptimizationParams)
# Compute the new CFG first (modulo statement ranges, which will be computed below)
state = CFGInliningState(ir)
for (idx, item) in todo
if isa(item, UnionSplit)
cfg_inline_unionsplit!(ir, idx, item::UnionSplit, state)
cfg_inline_unionsplit!(ir, idx, item::UnionSplit, state, params)
else
item = item::InliningTodo
spec = item.spec::ResolvedInliningSpec
Expand Down Expand Up @@ -599,7 +605,7 @@ function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vect
if isa(item, InliningTodo)
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs)
elseif isa(item, UnionSplit)
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs)
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs, params)
end
compact[idx] = nothing
refinish && finish_current_bb!(compact, 0)
Expand Down Expand Up @@ -845,7 +851,7 @@ end

function handle_single_case!(
ir::IRCode, idx::Int, stmt::Expr,
@nospecialize(case), todo::Vector{Pair{Int, Any}}, isinvoke::Bool = false)
@nospecialize(case), todo::Vector{Pair{Int, Any}}, params::OptimizationParams, isinvoke::Bool = false)
if isa(case, ConstantCase)
ir[SSAValue(idx)][:inst] = case.val
elseif isa(case, MethodInstance)
Expand Down Expand Up @@ -1017,12 +1023,12 @@ function inline_invoke!(
validate_sparams(mi.sparam_vals) || return nothing
if argtypes_to_type(argtypes) <: mi.def.sig
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
handle_single_case!(ir, idx, stmt, item, todo, true)
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
return nothing
end
end
item = analyze_method!(match, argtypes, flag, state)
handle_single_case!(ir, idx, stmt, item, todo, true)
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
return nothing
end

Expand Down Expand Up @@ -1190,7 +1196,7 @@ function analyze_single_call!(
fully_covered &= atype <: signature_union
end

handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo)
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo, state.params)
end

# similar to `analyze_single_call!`, but with constant results
Expand Down Expand Up @@ -1241,7 +1247,7 @@ function handle_const_call!(
fully_covered &= atype <: signature_union
end

handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo)
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo, state.params)
end

function handle_match!(
Expand Down Expand Up @@ -1270,12 +1276,13 @@ function handle_const_result!(
end

function handle_cases!(ir::IRCode, idx::Int, stmt::Expr, @nospecialize(atype),
cases::Vector{InliningCase}, fully_covered::Bool, todo::Vector{Pair{Int, Any}})
cases::Vector{InliningCase}, fully_covered::Bool, todo::Vector{Pair{Int, Any}},
params::OptimizationParams)
# If we only have one case and that case is fully covered, we may either
# be able to do the inlining now (for constant cases), or push it directly
# onto the todo list
if fully_covered && length(cases) == 1
handle_single_case!(ir, idx, stmt, cases[1].item, todo)
handle_single_case!(ir, idx, stmt, cases[1].item, todo, params)
elseif length(cases) > 0
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
end
Expand All @@ -1289,7 +1296,7 @@ function handle_const_opaque_closure_call!(
isdispatchtuple(item.mi.specTypes) || return
validate_sparams(item.mi.sparam_vals) || return
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
handle_single_case!(ir, idx, stmt, item, todo)
handle_single_case!(ir, idx, stmt, item, todo, state.params)
return nothing
end

Expand Down Expand Up @@ -1334,7 +1341,7 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
sig, state, todo)
else
item = analyze_method!(info.match, sig.argtypes, flag, state)
handle_single_case!(ir, idx, stmt, item, todo)
handle_single_case!(ir, idx, stmt, item, todo, state.params)
end
continue
end
Expand Down
6 changes: 5 additions & 1 deletion base/compiler/types.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ struct OptimizationParams
inline_tupleret_bonus::Int # extra inlining willingness for non-concrete tuple return types (in hopes of splitting it up)
inline_error_path_cost::Int # cost of (un-optimized) calls in blocks that throw

trust_inference::Bool

# Duplicating for now because optimizer inlining requires it.
# Keno assures me this will be removed in the near future
MAX_METHODS::Int
Expand All @@ -69,16 +71,18 @@ struct OptimizationParams
max_methods::Int = 3,
tuple_splat::Int = 32,
union_splitting::Int = 4,
trust_inference::Bool = false
)
return new(
inlining,
inline_cost_threshold,
inline_nonleaf_penalty,
inline_tupleret_bonus,
inline_error_path_cost,
trust_inference,
max_methods,
tuple_splat,
union_splitting,
union_splitting
)
end
end
Expand Down

0 comments on commit 8b8e338

Please sign in to comment.