inference: fixes and improvements for backedge computation

[aviatesk]

This PR consists of the following commits:

• inference: setup separate functions for each backedge kind
  Hopefully make it clear what kind of backedge is being added at each place.
  This commit also changes the argument list so that they are ordered as
  (caller, [backedge information]).

• inference: fix backedge computation for const-prop'ed callsite
  With this commit abstract_call_method_with_const_args doesn't add
  backedge but rather returns the backedge to the caller, letting the
  callers like abstract_call_gf_by_type and abstract_invoke take the
  responsibility to add backedge to current context appropriately.

  As a result, this fixes the backedge calculation for const-prop'ed
  invoke callsite.

  For example, for the following call graph,

  foo(a::Int) = a > 0 ? :int : println(a)
  foo(a::Integer) = a > 0 ? "integer" : println(a)
  
  bar(a::Int) = @invoke foo(a::Integer)

  Previously we added the wrong backedge nothing, bar(Int64) from bar(Int64):

  julia> last(only(code_typed(()->bar(42))))
  String
  
  julia> let m = only(methods(foo, (UInt,)))
             @eval Core.Compiler for (sig, caller) in BackedgeIterator($m.specializations[1].backedges)
                 println(sig, ", ", caller)
             end
         end
  Tuple{typeof(Main.foo), Integer}, bar(Int64) from bar(Int64)
  nothing, bar(Int64) from bar(Int64)

  but now we only add invoke-backedge:

  julia> last(only(code_typed(()->bar(42))))
  String
  
  julia> let m = only(methods(foo, (UInt,)))
             @eval Core.Compiler for (; sig, caller) in BackedgeIterator($m.specializations[1].backedges)
                 println(sig, ", ", caller)
             end
         end
  Tuple{typeof(Main.foo), Integer}, bar(Int64) from bar(Int64)

[aviatesk]

@nanosoldier runbenchmarks("inference", vs=":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

[aviatesk]

@nanosoldier runbenchmarks("inference", vs=":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

[Keno]

This is pretty subtle. Can we make sure to have a test case that actually triggers this invalidation to make sure that the invoke backedge exists and works properly?

[timholy]

Really appreciate you doing this! I think you're doing a better job of implementing a transition I was wondering about, whether we need to start considering invokesig an obligate argument; in particular, filling in a default value of nothing seems to be a formula for inadvertently dropping it. Here's another example exploiting

julia/base/multidimensional.jl

Line 1642 in ffaaa30

_unique_dims(A::AbstractArray, dims::Colon) = invoke(unique, Tuple{Any}, A)

that unfortunately still fails on this branch:

julia> m = which(unique, Tuple{Any})
unique(itr)
     @ Base set.jl:170

julia> specs = collect(Iterators.filter(m.specializations) do mi
           mi === nothing && return false
           return mi.specTypes.parameters[end] === Vector{Int}   # find specialization of `unique(::Any)` for `::Vector{Int}`
       end)
Any[]

julia> Base._unique_dims([1,2,3],:)   # no existing callers with specialization `Vector{Int}`, let's make one
3-element Vector{Int64}:
 1
 2
 3

julia> mi = only(Iterators.filter(m.specializations) do mi
           mi === nothing && return false
           return mi.specTypes.parameters[end] === Vector{Int}   # find specialization of `unique(::Any)` for `::Vector{Int}`
       end)
MethodInstance for unique(::Vector{Int64})

julia> mi.def
unique(itr)
     @ Base set.jl:170

julia> mi.backedges
3-element Vector{Any}:
 Tuple{typeof(unique), Any}
 MethodInstance for Base._unique_dims(::Vector{Int64}, ::Colon)
 MethodInstance for Base._unique_dims(::Vector{Int64}, ::Colon)

The MethodInstance should not be duplicated. I/we could fix it like #46715 but I wonder if more will crop up.

[aviatesk]

in particular, filling in a default value of nothing seems to be a formula for inadvertently dropping it. Here's another example exploiting

The last commit should fix this case. It turns out that we also need to fixup backedge calculation during inlining.

In general, I think the current implementation is maintainable. At least now we distinguish usual/abstract/invoke backedges, that hopefully has an essentially same effects as making invokesig required argument all over the place.

[aviatesk]

@Keno why do we need to account for mi-backedge for this constant result case? The mi is retrieved here

julia/base/compiler/ssair/inlining.jl

Line 954 in 59daacb

mi = specialize_method(match; preexisting=true) # Union{Nothing, MethodInstance}

and so the abstract interpretation should account for it already.

[Keno]

We were probably just being conservative here. We didn't used to be particularly careful about whether information was being computed during abstract interpretation or in the optimizer.

[aviatesk]

I see. It seems a bit inconsistent that we doesn't account for mi-backedge for [semi-]concrete evaluated case, e.g.:

julia/base/compiler/ssair/inlining.jl

Lines 1464 to 1474 in 59daacb

	function concrete_result_item(result::ConcreteResult, state::InliningState, @nospecialize(invokesig=nothing))
	if !isdefined(result, :result) || !is_inlineable_constant(result.result)
	et = InliningEdgeTracker(state.et, invokesig)
	case = compileable_specialization(result.mi, result.effects, et;
	compilesig_invokes=state.params.compilesig_invokes)
	@assert case !== nothing "concrete evaluation should never happen for uncompileable callsite"
	return case
	end
	@assert result.effects === EFFECTS_TOTAL
	return ConstantCase(quoted(result.result))
	end

[Keno]

Well, come up with something that has a constant return that gets inlined here, add a test for it and then delete the backedge here. If things pass (and go through PkgEval), I'm comfortable to remove it.