inlining: Use concrete-eval effects if available

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