Skip to content

Commit 2118a08

Browse files
authored
Add an option to not emit unionsplit fallback blocks (#43923)
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.
1 parent 0bdf24f commit 2118a08

File tree

2 files changed

+46
-35
lines changed

2 files changed

+46
-35
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ function ssa_inlining_pass!(ir::IRCode, linetable::Vector{LineInfoNode}, state::
7575
@timeit "analysis" todo = assemble_inline_todo!(ir, state)
7676
isempty(todo) && return ir
7777
# Do the actual inlining for every call we identified
78-
@timeit "execution" ir = batch_inline!(todo, ir, linetable, propagate_inbounds)
78+
@timeit "execution" ir = batch_inline!(todo, ir, linetable, propagate_inbounds, state.params)
7979
return ir
8080
end
8181

@@ -210,7 +210,8 @@ end
210210

211211
function cfg_inline_unionsplit!(ir::IRCode, idx::Int,
212212
(; fully_covered, #=atype,=# cases, bbs)::UnionSplit,
213-
state::CFGInliningState)
213+
state::CFGInliningState,
214+
params::OptimizationParams)
214215
inline_into_block!(state, block_for_inst(ir, idx))
215216
from_bbs = Int[]
216217
delete!(state.split_targets, length(state.new_cfg_blocks))
@@ -233,7 +234,7 @@ function cfg_inline_unionsplit!(ir::IRCode, idx::Int,
233234
push!(from_bbs, length(state.new_cfg_blocks))
234235
# TODO: Right now we unconditionally generate a fallback block
235236
# in case of subtyping errors - This is probably unnecessary.
236-
if true # i != length(cases) || !fully_covered
237+
if i != length(cases) || (!fully_covered || !params.trust_inference)
237238
# This block will have the next condition or the final else case
238239
push!(state.new_cfg_blocks, BasicBlock(StmtRange(idx, idx)))
239240
push!(state.new_cfg_blocks[cond_bb].succs, length(state.new_cfg_blocks))
@@ -457,12 +458,13 @@ const FATAL_TYPE_BOUND_ERROR = ErrorException("fatal error in type inference (ty
457458
function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
458459
argexprs::Vector{Any}, linetable::Vector{LineInfoNode},
459460
(; fully_covered, atype, cases, bbs)::UnionSplit,
460-
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
461+
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
462+
params::OptimizationParams)
461463
stmt, typ, line = compact.result[idx][:inst], compact.result[idx][:type], compact.result[idx][:line]
462464
join_bb = bbs[end]
463465
pn = PhiNode()
464466
local bb = compact.active_result_bb
465-
@assert length(bbs) > length(cases)
467+
@assert length(bbs) >= length(cases)
466468
for i in 1:length(cases)
467469
ithcase = cases[i]
468470
metharg = ithcase.sig
@@ -472,21 +474,23 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
472474
cond = true
473475
aparams, mparams = atype.parameters::SimpleVector, metharg.parameters::SimpleVector
474476
@assert length(aparams) == length(mparams)
475-
for i in 1:length(aparams)
476-
a, m = aparams[i], mparams[i]
477-
# If this is always true, we don't need to check for it
478-
a <: m && continue
479-
# Generate isa check
480-
isa_expr = Expr(:call, isa, argexprs[i], m)
481-
ssa = insert_node_here!(compact, NewInstruction(isa_expr, Bool, line))
482-
if cond === true
483-
cond = ssa
484-
else
485-
and_expr = Expr(:call, and_int, cond, ssa)
486-
cond = insert_node_here!(compact, NewInstruction(and_expr, Bool, line))
477+
if i != length(cases) || !fully_covered || !params.trust_inference
478+
for i in 1:length(aparams)
479+
a, m = aparams[i], mparams[i]
480+
# If this is always true, we don't need to check for it
481+
a <: m && continue
482+
# Generate isa check
483+
isa_expr = Expr(:call, isa, argexprs[i], m)
484+
ssa = insert_node_here!(compact, NewInstruction(isa_expr, Bool, line))
485+
if cond === true
486+
cond = ssa
487+
else
488+
and_expr = Expr(:call, and_int, cond, ssa)
489+
cond = insert_node_here!(compact, NewInstruction(and_expr, Bool, line))
490+
end
487491
end
492+
insert_node_here!(compact, NewInstruction(GotoIfNot(cond, next_cond_bb), Union{}, line))
488493
end
489-
insert_node_here!(compact, NewInstruction(GotoIfNot(cond, next_cond_bb), Union{}, line))
490494
bb = next_cond_bb - 1
491495
finish_current_bb!(compact, 0)
492496
argexprs′ = argexprs
@@ -525,10 +529,12 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
525529
bb += 1
526530
# We're now in the fall through block, decide what to do
527531
if fully_covered
528-
e = Expr(:call, GlobalRef(Core, :throw), FATAL_TYPE_BOUND_ERROR)
529-
insert_node_here!(compact, NewInstruction(e, Union{}, line))
530-
insert_node_here!(compact, NewInstruction(ReturnNode(), Union{}, line))
531-
finish_current_bb!(compact, 0)
532+
if !params.trust_inference
533+
e = Expr(:call, GlobalRef(Core, :throw), FATAL_TYPE_BOUND_ERROR)
534+
insert_node_here!(compact, NewInstruction(e, Union{}, line))
535+
insert_node_here!(compact, NewInstruction(ReturnNode(), Union{}, line))
536+
finish_current_bb!(compact, 0)
537+
end
532538
else
533539
ssa = insert_node_here!(compact, NewInstruction(stmt, typ, line))
534540
push!(pn.edges, bb)
@@ -541,12 +547,12 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
541547
return insert_node_here!(compact, NewInstruction(pn, typ, line))
542548
end
543549

544-
function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vector{LineInfoNode}, propagate_inbounds::Bool)
550+
function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vector{LineInfoNode}, propagate_inbounds::Bool, params::OptimizationParams)
545551
# Compute the new CFG first (modulo statement ranges, which will be computed below)
546552
state = CFGInliningState(ir)
547553
for (idx, item) in todo
548554
if isa(item, UnionSplit)
549-
cfg_inline_unionsplit!(ir, idx, item::UnionSplit, state)
555+
cfg_inline_unionsplit!(ir, idx, item::UnionSplit, state, params)
550556
else
551557
item = item::InliningTodo
552558
spec = item.spec::ResolvedInliningSpec
@@ -599,7 +605,7 @@ function batch_inline!(todo::Vector{Pair{Int, Any}}, ir::IRCode, linetable::Vect
599605
if isa(item, InliningTodo)
600606
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs)
601607
elseif isa(item, UnionSplit)
602-
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs)
608+
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, linetable, item, boundscheck, state.todo_bbs, params)
603609
end
604610
compact[idx] = nothing
605611
refinish && finish_current_bb!(compact, 0)
@@ -845,7 +851,7 @@ end
845851

846852
function handle_single_case!(
847853
ir::IRCode, idx::Int, stmt::Expr,
848-
@nospecialize(case), todo::Vector{Pair{Int, Any}}, isinvoke::Bool = false)
854+
@nospecialize(case), todo::Vector{Pair{Int, Any}}, params::OptimizationParams, isinvoke::Bool = false)
849855
if isa(case, ConstantCase)
850856
ir[SSAValue(idx)][:inst] = case.val
851857
elseif isa(case, MethodInstance)
@@ -1017,12 +1023,12 @@ function inline_invoke!(
10171023
validate_sparams(mi.sparam_vals) || return nothing
10181024
if argtypes_to_type(argtypes) <: mi.def.sig
10191025
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1020-
handle_single_case!(ir, idx, stmt, item, todo, true)
1026+
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
10211027
return nothing
10221028
end
10231029
end
10241030
item = analyze_method!(match, argtypes, flag, state)
1025-
handle_single_case!(ir, idx, stmt, item, todo, true)
1031+
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
10261032
return nothing
10271033
end
10281034

@@ -1190,7 +1196,7 @@ function analyze_single_call!(
11901196
fully_covered &= atype <: signature_union
11911197
end
11921198

1193-
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo)
1199+
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo, state.params)
11941200
end
11951201

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

1244-
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo)
1250+
handle_cases!(ir, idx, stmt, atype, cases, fully_covered, todo, state.params)
12451251
end
12461252

12471253
function handle_match!(
@@ -1270,12 +1276,13 @@ function handle_const_result!(
12701276
end
12711277

12721278
function handle_cases!(ir::IRCode, idx::Int, stmt::Expr, @nospecialize(atype),
1273-
cases::Vector{InliningCase}, fully_covered::Bool, todo::Vector{Pair{Int, Any}})
1279+
cases::Vector{InliningCase}, fully_covered::Bool, todo::Vector{Pair{Int, Any}},
1280+
params::OptimizationParams)
12741281
# If we only have one case and that case is fully covered, we may either
12751282
# be able to do the inlining now (for constant cases), or push it directly
12761283
# onto the todo list
12771284
if fully_covered && length(cases) == 1
1278-
handle_single_case!(ir, idx, stmt, cases[1].item, todo)
1285+
handle_single_case!(ir, idx, stmt, cases[1].item, todo, params)
12791286
elseif length(cases) > 0
12801287
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
12811288
end
@@ -1289,7 +1296,7 @@ function handle_const_opaque_closure_call!(
12891296
isdispatchtuple(item.mi.specTypes) || return
12901297
validate_sparams(item.mi.sparam_vals) || return
12911298
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1292-
handle_single_case!(ir, idx, stmt, item, todo)
1299+
handle_single_case!(ir, idx, stmt, item, todo, state.params)
12931300
return nothing
12941301
end
12951302

@@ -1334,7 +1341,7 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
13341341
sig, state, todo)
13351342
else
13361343
item = analyze_method!(info.match, sig.argtypes, flag, state)
1337-
handle_single_case!(ir, idx, stmt, item, todo)
1344+
handle_single_case!(ir, idx, stmt, item, todo, state.params)
13381345
end
13391346
continue
13401347
end

base/compiler/types.jl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ struct OptimizationParams
5454
inline_tupleret_bonus::Int # extra inlining willingness for non-concrete tuple return types (in hopes of splitting it up)
5555
inline_error_path_cost::Int # cost of (un-optimized) calls in blocks that throw
5656

57+
trust_inference::Bool
58+
5759
# Duplicating for now because optimizer inlining requires it.
5860
# Keno assures me this will be removed in the near future
5961
MAX_METHODS::Int
@@ -69,16 +71,18 @@ struct OptimizationParams
6971
max_methods::Int = 3,
7072
tuple_splat::Int = 32,
7173
union_splitting::Int = 4,
74+
trust_inference::Bool = false
7275
)
7376
return new(
7477
inlining,
7578
inline_cost_threshold,
7679
inline_nonleaf_penalty,
7780
inline_tupleret_bonus,
7881
inline_error_path_cost,
82+
trust_inference,
7983
max_methods,
8084
tuple_splat,
81-
union_splitting,
85+
union_splitting
8286
)
8387
end
8488
end

0 commit comments

Comments
 (0)