Inliner cost model: don't count :unreachable branches

[timholy]

This improves the heuristic for inlining by not penalizing branches that end with a throw. The idea is that exception paths don't reflect the typical run-time cost of a function, since generally one shouldn't be using exceptions for control-flow.

I have some local data suggesting this is helpful, but let's let nanosoldier chime in:
@nanosoldier runbenchmarks(ALL, vs = ":master")

Overall this seemed suspiciously easy, so I'm guessing I'm missing something. Would love to have input from someone who knows more.

Replaces #23986. Ref #29798; closing that also requires that we adjust the cost model for some ccalls that end up being special-cased by the compiler. I haven't yet invested the time to figure out exactly how those should be modeled.

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[timholy]

I'm wondering if I'm encountering a compiler bug. I stumbled across this while looking into regressions when I disable @inline annotations (it is based on one of the "union" tests in BaseBenchmarks):

const VEC_LENGTH = 10000
T = BigInt
S = Int128
X2 = Vector{Union{T, Nothing}}(Vector{T}(rand(S, VEC_LENGTH)))
Y2 = Vector{Union{T, Nothing}}(Vector{T}(rand(S, VEC_LENGTH)))
X2[rand(VEC_LENGTH) .> .9] .= nothing
Y2[rand(VEC_LENGTH) .> .9] .= nothing

_mul(x, y) = x * y
_mul(::Nothing, ::Any) = nothing
_mul(::Any, ::Nothing) = nothing
_mul(::Nothing, ::Nothing) = nothing

g = Base.Generator(_mul, X2, Y2)

@code_warntype iterate(g)

When I do this on master, I get nice union-splitting:

   17 ┄ %42 = φ (#6 => true, #15 => true, #16 => false)::Bool       │││      
   │    %43 = φ (#16 => %37)::Union{Nothing, BigInt}                │││      
   │    %44 = φ (#16 => %38)::Int64                                 │││      
   │    %45 = φ (#16 => %14)::Union{Nothing, BigInt}                │││      
   │    %46 = φ (#16 => %15)::Int64                                 │││      
   └───       goto #19 if not %42                                   │││      
   18 ─       goto #20                                              │││      
   19 ─ %49 = (Core.tuple)(%46, %44)::Tuple{Int64,Int64}            │││╻        _zip_iterate_interleave
   └───       goto #20                                              │││      
   20 ┄ %51 = φ (#18 => true, #19 => false)::Bool                   ││       
   │    %52 = φ (#19 => %45)::Union{Nothing, BigInt}                ││       
   │    %53 = φ (#19 => %43)::Union{Nothing, BigInt}                ││       
   │    %54 = φ (#19 => %49)::Tuple{Int64,Int64}                    ││       
   └───       goto #21                                              ││       
45 21 ─       goto #23 if not %51                                   │        
   22 ─       return Base.nothing                                   │        
47 23 ─ %58 = (isa)(%52, Nothing)::Bool                             │╻        #3
   │    %59 = (isa)(%53, Nothing)::Bool                             ││       
   │    %60 = (and_int)(%58, %59)::Bool                             ││       
   └───       goto #25 if not %60                                   ││       
   24 ─       goto #32                                              ││       
   25 ─ %63 = (isa)(%52, BigInt)::Bool                              ││       
   │    %64 = (isa)(%53, Nothing)::Bool                             ││       
   │    %65 = (and_int)(%63, %64)::Bool                             ││       
...

But on this branch, I get the following:

   17 ┄ %42 = φ (#6 => true, #15 => true, #16 => false)::Bool      │││      
   │    %43 = φ (#16 => %37)::Union{Nothing, BigInt}               │││      
   │    %44 = φ (#16 => %38)::Int64                                │││      
   │    %45 = φ (#16 => %14)::Union{Nothing, BigInt}               │││      
   │    %46 = φ (#16 => %15)::Int64                                │││      
   └───       goto #19 if not %42                                  │││      
   18 ─       goto #20                                             │││      
   19 ─ %49 = (Core.tuple)(%45, %43)::Tuple{Union{Nothing, BigInt},Union{Nothing, BigInt}}e_interleave
   │    %50 = (Core.tuple)(%46, %44)::Tuple{Int64,Int64}           ││││     
   └───       goto #20                                             │││      
   20 ┄ %52 = φ (#18 => true, #19 => false)::Bool                  ││       
   │    %53 = φ (#19 => %49)::Tuple{Union{Nothing, BigInt},Union{Nothing, BigInt}}
   │    %54 = φ (#19 => %50)::Tuple{Int64,Int64}                   ││       
   └───       goto #21                                             ││       
45 21 ─       goto #23 if not %52                                  │        
   22 ─       return Base.nothing                                  │        
47 23 ─ %58 = (getfield(Base, Symbol("##3#4")){typeof(_mul)}(_mul))(%53)::Union{Nothing, BigInt}
   │    %59 = (Core.tuple)(%58, %54)::Tuple{Union{Nothing, BigInt},Tuple{Int64,Int64}}
   └───       return %59                                           │        

(note I truncated the output from master but included it all the way to the end here).

That %58 line seems weird: it appears that _mul has been turned into an anonymous function taking a 2-tuple (%49, not created on master), as opposed to 2 arguments. If I analyze how show is working, detecting what this is seems somewhat arcane:

ci = code_typed(iterate, Tuple{typeof(g)})[1].first

julia> ex = ci.code[end-2]
:((getfield(Base, Symbol("##3#4")){typeof(_mul)}(_mul))(%53))

julia> f = ex.args[1]
#3 (generic function with 1 method)

julia> tn = typeof(f).name
getfield(Base, Symbol("##3#4"))

julia> startswith(string(tn.name), '#')
true

and the fact that this returns true is apparently what indicates that this is a getfield call.

Now, I don't understand why that getfield gets elided on master but not on this branch: this branch seems like it should only affect inlining, and be strictly more permissive than master. I guess I'm wondering if the somewhat arcane nature of detecting the getfield call is the root cause of a bug to fail to detect it elsewhere in inference. (Note there is one such attempt even in statement_cost; not sure if that's ever valid anymore.)

[JeffBezanson]

I don't think there is actually a getfield call there; that's just how it's printed so that the qualified name is valid entry syntax, i.e. not Base.##3#4.

[timholy]

Interesting. So your suggestion is this is actually a function call that was perhaps deemed ineligible for inlining? If so why is it not invoke?

[oscardssmith]

Would this still be helpful?