Skip to content

Commit ead2227

Browse files
committed
concrete-eval: Use concrete eval information even if the result is too large
When we originally implemented concrete-eval, we decided not to create `Const` lattice elements for constants that are too large, on the fear that these would end up in the IR and blowing up the size. Now that we have some experience with this, I think that decision was incorrect for several reasons: 1. We've already performed the concrete evaluation (and allocated the big object), so we're just throwing away precision here that we could have otherwise used (Although if the call result is unused, we probably shouldn't do concrete eval at all - see #46774). 2. There's a number of other places in the compiler where we read large values into `Const`. Unless we add these kinds of check there too, we need to have appropriate guards in the optimizer and the cache anyway, to prevent the IR size blowup. 3. It turns out that throwing away this precision actually causes significant performance problems for code that is just over the line. Consider for example a lookup of a small struct inside a large, multi-level constant structure. The final result might be quite tiny, but if we refuse to propagate the intermediate levels, we might end up doing an (expensive) full-constprop when propagating the constant information could have given us a much cheaper concrete evaluation. This commit simply removes that check. If we see any regressions as a result of this, we should see if there are additional guards needed in the optimizer.
1 parent ca3cd8b commit ead2227

File tree

2 files changed

+12
-17
lines changed

2 files changed

+12
-17
lines changed

base/compiler/abstractinterpretation.jl

+1-7
Original file line numberDiff line numberDiff line change
@@ -841,13 +841,7 @@ function concrete_eval_call(interp::AbstractInterpreter,
841841
# The evaluation threw. By :consistent-cy, we're guaranteed this would have happened at runtime
842842
return ConstCallResults(Union{}, ConcreteResult(edge, result.effects), result.effects, edge)
843843
end
844-
if is_inlineable_constant(value) || call_result_unused(si)
845-
# If the constant is not inlineable, still do the const-prop, since the
846-
# code that led to the creation of the Const may be inlineable in the same
847-
# circumstance and may be optimizable.
848-
return ConstCallResults(Const(value), ConcreteResult(edge, EFFECTS_TOTAL, value), EFFECTS_TOTAL, edge)
849-
end
850-
return false
844+
return ConstCallResults(Const(value), ConcreteResult(edge, EFFECTS_TOTAL, value), EFFECTS_TOTAL, edge)
851845
else # eligible for semi-concrete evaluation
852846
return true
853847
end

base/compiler/ssair/inlining.jl

+11-10
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
11691169
end
11701170
result = info.result
11711171
invokesig = sig.argtypes
1172-
if isa(result, ConcreteResult)
1172+
if isa(result, ConcreteResult) && may_inline_concrete_result(result)
11731173
item = concrete_result_item(result, state; invokesig)
11741174
else
11751175
argtypes = invoke_rewrite(sig.argtypes)
@@ -1296,7 +1296,11 @@ function handle_any_const_result!(cases::Vector{InliningCase},
12961296
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
12971297
allow_abstract::Bool, allow_typevars::Bool)
12981298
if isa(result, ConcreteResult)
1299-
return handle_concrete_result!(cases, result, state)
1299+
if may_inline_concrete_result(result)
1300+
return handle_concrete_result!(cases, result, state)
1301+
else
1302+
result = nothing
1303+
end
13001304
end
13011305
if isa(result, SemiConcreteResult)
13021306
result = inlining_policy(state.interp, result, info, flag, result.mi, argtypes)
@@ -1483,15 +1487,12 @@ function handle_concrete_result!(cases::Vector{InliningCase}, result::ConcreteRe
14831487
return true
14841488
end
14851489

1490+
may_inline_concrete_result(result::ConcreteResult) =
1491+
isdefined(result, :result) && is_inlineable_constant(result.result)
1492+
14861493
function concrete_result_item(result::ConcreteResult, state::InliningState;
14871494
invokesig::Union{Nothing,Vector{Any}}=nothing)
1488-
if !isdefined(result, :result) || !is_inlineable_constant(result.result)
1489-
et = InliningEdgeTracker(state.et, invokesig)
1490-
case = compileable_specialization(result.mi, result.effects, et;
1491-
compilesig_invokes=state.params.compilesig_invokes)
1492-
@assert case !== nothing "concrete evaluation should never happen for uncompileable callsite"
1493-
return case
1494-
end
1495+
@assert may_inline_concrete_result(result)
14951496
@assert result.effects === EFFECTS_TOTAL
14961497
return ConstantCase(quoted(result.result))
14971498
end
@@ -1524,7 +1525,7 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
15241525
mi = result.result.linfo
15251526
validate_sparams(mi.sparam_vals) || return nothing
15261527
item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
1527-
elseif isa(result, ConcreteResult)
1528+
elseif isa(result, ConcreteResult) && may_inline_concrete_result(result)
15281529
item = concrete_result_item(result, state)
15291530
else
15301531
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)

0 commit comments

Comments
 (0)