inlining: Use concrete-eval effects if available (#47305)

It is possible for concrete-eval to refine effects, but be unable
to inline (because the constant is too large, c.f. #47283).
However, in that case, we would still like to use the extra
effect information to make sure that the optimizer can delete
the statement if it turns out to be unused. There are two cases:
the first is where the call is not inlineable at all. This
one is simple, because we just apply the effects on the :invoke
as we usually would. The second is trickier: If we do end up
inlining the call, we need to apply the overriden effects to
every inlined statement, because we lose the identity of the
function as a whole. This is a bit nasty and I don't really like
it, but I'm not sure what a better alternative would be. We could
always refuse to inline calls with large-constant results
(since we currently pessimize what is being inlined anyway), but
I'm not sure that would be better. This is a simple solution and
works for the case I have in practice, but we may want to revisit
it in the future.

Author: Keno

base/compiler/ssair/inlining.jl

@@ -370,7 +370,8 @@ end
 
 function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector{Any},
                          linetable::Vector{LineInfoNode}, item::InliningTodo,
-                         boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
+                         boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
+                         extra_flags::UInt8 = inlined_flags_for_effects(item.effects))
     # Ok, do the inlining here
     sparam_vals = item.mi.sparam_vals
     def = item.mi.def::Method
@@ -411,6 +412,7 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
                 break
             end
             inline_compact[idx′] = stmt′
+            inline_compact[SSAValue(idx′)][:flag] |= extra_flags
         end
         just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
         compact.result_idx = inline_compact.result_idx
@@ -445,6 +447,14 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
                 stmt′ = PhiNode(Int32[edge+bb_offset for edge in stmt′.edges], stmt′.values)
             end
             inline_compact[idx′] = stmt′
+            if extra_flags != 0 && !isa(stmt′, Union{GotoNode, GotoIfNot})
+                if (extra_flags & IR_FLAG_NOTHROW) != 0 && inline_compact[SSAValue(idx′)][:type] === Union{}
+                    # Shown nothrow, but also guaranteed to throw => unreachable
+                    inline_compact[idx′] = ReturnNode()
+                else
+                    inline_compact[SSAValue(idx′)][:flag] |= extra_flags
+                end
+            end
         end
         just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
         compact.result_idx = inline_compact.result_idx
@@ -838,8 +848,9 @@ end
 
 # the general resolver for usual and const-prop'ed calls
 function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceResult},
-    argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
-    state::InliningState; invokesig::Union{Nothing,Vector{Any}}=nothing)
+        argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
+        state::InliningState; invokesig::Union{Nothing,Vector{Any}}=nothing,
+        override_effects::Effects = EFFECTS_UNKNOWN′)
     et = InliningEdgeTracker(state.et, invokesig)
 
     #XXX: update_valid_age!(min_valid[1], max_valid[1], sv)
@@ -860,6 +871,10 @@ function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceRes
         (; src, effects) = cached_result
     end
 
+    if override_effects !== EFFECTS_UNKNOWN′
+        effects = override_effects
+    end
+
     # the duplicated check might have been done already within `analyze_method!`, but still
     # we need it here too since we may come here directly using a constant-prop' result
     if !state.params.inlining || is_stmt_noinline(flag)
@@ -937,7 +952,8 @@ can_inline_typevars(m::MethodMatch, argtypes::Vector{Any}) = can_inline_typevars
 
 function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
     @nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
-    allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing)
+    allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing,
+    override_effects::Effects=EFFECTS_UNKNOWN′)
     method = match.method
     spec_types = match.spec_types
 
@@ -967,11 +983,13 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
     mi = specialize_method(match; preexisting=true) # Union{Nothing, MethodInstance}
     if mi === nothing
         et = InliningEdgeTracker(state.et, invokesig)
-        return compileable_specialization(match, Effects(), et, info;
+        effects = override_effects
+        effects === EFFECTS_UNKNOWN′ && (effects = info_effects(nothing, match, state))
+        return compileable_specialization(match, effects, et, info;
             compilesig_invokes=state.params.compilesig_invokes)
     end
 
-    return resolve_todo(mi, match, argtypes, info, flag, state; invokesig)
+    return resolve_todo(mi, match, argtypes, info, flag, state; invokesig, override_effects)
 end
 
 function retrieve_ir_for_inlining(mi::MethodInstance, src::Array{UInt8, 1})
@@ -994,6 +1012,37 @@ function flags_for_effects(effects::Effects)
     return flags
 end
 
+"""
+    inlined_flags_for_effects(effects::Effects)
+
+This function answers the query:
+
+    Given a call site annotated as `effects`, what can we say about each inlined
+    statement after the inlining?
+
+Note that this is different from `flags_for_effects`, which just talks about
+the call site itself. Consider for example:
+
+````
+    function foo()
+        V = Any[]
+        push!(V, 1)
+        tuple(V...)
+    end
+```
+
+This function is properly inferred effect_free, because it has no global effects.
+However, we may not inline each statement with an :effect_free flag, because
+that would incorrectly lose the `push!`.
+"""
+function inlined_flags_for_effects(effects::Effects)
+    flags::UInt8 = 0
+    if is_nothrow(effects)
+        flags |= IR_FLAG_NOTHROW
+    end
+    return flags
+end
+
 function handle_single_case!(todo::Vector{Pair{Int,Any}},
     ir::IRCode, idx::Int, stmt::Expr, @nospecialize(case), params::OptimizationParams,
     isinvoke::Bool = false)
@@ -1170,21 +1219,26 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
     end
     result = info.result
     invokesig = sig.argtypes
-    if isa(result, ConcreteResult) && may_inline_concrete_result(result)
-        item = concrete_result_item(result, state; invokesig)
-    else
-        argtypes = invoke_rewrite(sig.argtypes)
-        if isa(result, ConstPropResult)
-            mi = result.result.linfo
-            validate_sparams(mi.sparam_vals) || return nothing
-            if argtypes_to_type(argtypes) <: mi.def.sig
-                item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig)
-                handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
-                return nothing
-            end
+    override_effects = EFFECTS_UNKNOWN′
+    if isa(result, ConcreteResult)
+        if may_inline_concrete_result(result)
+            item = concrete_result_item(result, state; invokesig)
+            handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
+            return nothing
         end
-        item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig)
+        override_effects = result.effects
     end
+    argtypes = invoke_rewrite(sig.argtypes)
+    if isa(result, ConstPropResult)
+        mi = result.result.linfo
+        validate_sparams(mi.sparam_vals) || return nothing
+        if argtypes_to_type(argtypes) <: mi.def.sig
+            item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig, override_effects)
+            handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
+            return nothing
+        end
+    end
+    item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, override_effects)
     handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
     return nothing
 end
@@ -1296,10 +1350,12 @@ function handle_any_const_result!(cases::Vector{InliningCase},
     @nospecialize(result), match::MethodMatch, argtypes::Vector{Any},
     @nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
     allow_abstract::Bool, allow_typevars::Bool)
+    override_effects = EFFECTS_UNKNOWN′
     if isa(result, ConcreteResult)
         if may_inline_concrete_result(result)
             return handle_concrete_result!(cases, result, state)
         else
+            override_effects = result.effects
             result = nothing
         end
     end
@@ -1313,7 +1369,7 @@ function handle_any_const_result!(cases::Vector{InliningCase},
         return handle_const_prop_result!(cases, result, argtypes, info, flag, state; allow_abstract, allow_typevars)
     else
         @assert result === nothing
-        return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars)
+        return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars, override_effects)
     end
 end
 
@@ -1444,14 +1500,14 @@ end
 function handle_match!(cases::Vector{InliningCase},
     match::MethodMatch, argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
     state::InliningState;
-    allow_abstract::Bool, allow_typevars::Bool)
+    allow_abstract::Bool, allow_typevars::Bool, override_effects::Effects)
     spec_types = match.spec_types
     allow_abstract || isdispatchtuple(spec_types) || return false
     # We may see duplicated dispatch signatures here when a signature gets widened
     # during abstract interpretation: for the purpose of inlining, we can just skip
     # processing this dispatch candidate (unless unmatched type parameters are present)
     !allow_typevars && _any(case->case.sig === spec_types, cases) && return true
-    item = analyze_method!(match, argtypes, info, flag, state; allow_typevars)
+    item = analyze_method!(match, argtypes, info, flag, state; allow_typevars, override_effects)
     item === nothing && return false
     push!(cases, InliningCase(spec_types, item))
     return true
@@ -1526,8 +1582,13 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
         mi = result.result.linfo
         validate_sparams(mi.sparam_vals) || return nothing
         item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
-    elseif isa(result, ConcreteResult) && may_inline_concrete_result(result)
-        item = concrete_result_item(result, state)
+    elseif isa(result, ConcreteResult)
+        if may_inline_concrete_result(result)
+            item = concrete_result_item(result, state)
+        else
+            override_effects = result.effects
+            item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, override_effects)
+        end
     else
         item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)
     end

test/compiler/inline.jl

@@ -1811,3 +1811,28 @@ let src = code_typed1((NewInstruction,Any,Any,CallInfo)) do newinst, stmt, type,
     @test count(iscall((src,NamedTuple)), src.code) == 0
     @test count(isnew, src.code) == 1
 end
+
+# Test that inlining can still use nothrow information from concrete-eval
+# even if the result itself is too big to be inlined, and nothrow is not
+# known without concrete-eval
+const THE_BIG_TUPLE = ntuple(identity, 1024)
+function return_the_big_tuple(err::Bool)
+    err && error("BAD")
+    return THE_BIG_TUPLE
+end
+@noinline function return_the_big_tuple_noinline(err::Bool)
+    err && error("BAD")
+    return THE_BIG_TUPLE
+end
+big_tuple_test1() = return_the_big_tuple(false)[1]
+big_tuple_test2() = return_the_big_tuple_noinline(false)[1]
+
+@test fully_eliminated(big_tuple_test2, Tuple{})
+# Currently we don't run these cleanup passes, but let's make sure that
+# if we did, inlining would be able to remove this
+let ir = Base.code_ircode(big_tuple_test1, Tuple{})[1][1]
+    ir = Core.Compiler.compact!(ir, true)
+    ir = Core.Compiler.cfg_simplify!(ir)
+    ir = Core.Compiler.compact!(ir, true)
+    @test length(ir.stmts) == 1
+end