inference: parameterize some of hard-coded inference logic

[aviatesk]

This commit parameterizes some of hard-coded inference logic:

• to bail out from inference when a lattice element can't be refined or
  a current inference frame is proven to throw or to be dead
• to add call backedges when the call return type won't be refined

Those AbstractInterpreters used for code optimization (including
NativeInterpreter) usually just want the methods defined for
AbstractInterpreter, but some other AbstractInterpreter may want
other implementations and heuristics to control inference process.
For example, JETInterpreter is
used for code analysis and wants to add call backedges even when a call
return type is Any.

[aviatesk]

@vtjnash @Keno @JeffBezanson could I hear your ideas on this ?
This PR won't change any behavior of NativeInterpreter but can help other AbstractIntepreters tweak inference process more easily (otherwise they have to overload the entire body of abstract_call_gf_by_type etc.).
I'd really appreciate if we could merge these changes (unless I'm actually doing something wrong)

[aviatesk]

bump, @vtjnash @Keno @JeffBezanson

[Keno]

I think this is fine in general. Of course eventually, we'll probably want to find stable APIs for these sorts for things, but I'm fine with just refactoring in these hooks for now until we figure out what those APIs are.

[Keno]

What's the difference between bail_out_statement and bail_out_local?

[aviatesk]

bail_out_local controls the frame-level bail out logic in typeinf_local while bail_out_statement does the statement-level bail out logic in abstract_call_statement.
The reason because I separate them is that it gives us more fine-grained control on the heuristic and we may want to use it.

[Keno]

Upon further reflection, I'm not sure about this one. The return Bottom here kinda implies that the only thing this could possibly do is to check for a literal Bottom, no? Maybe it's a similar situation where the bail check needs to be separate from the check for Bottom.

[aviatesk]

I still think this could be useful for code analysis (but not in terms of optimization).

Say if we analyze this kind of code below:

function foo(a)
    a = a * missing # => NoMethodError
    sin_broad(a)
end

sin_broad(a) = sinn.(a) # => UndefVarError

foo([1,2,3]) # entry point

I believe it's better to report two errors in this case (i.e. NoMethodError(*, (Vector{Int}, Missing)), UndefVarError(:sinn))).
With the bail_out_statement and bail_out_local interface, we can do something like below:

import Core.Compiler:
    bail_out_local,
    bail_out_statement,
    abstract_call

bail_out_local(interp::JETInterpreter, @nospecialize(t), sv) = false
bail_out_statement(interp::JETInterpreter, @nospecialize(t), sv) = false

function CC.abstract_call(interp::JETInterpreter, ea::Union{Nothing,Vector{Any}}, argtypes::Vector{Any},
                          sv::InferenceState, max_methods::Int = InferenceParams(interp).MAX_METHODS)
    argtypes = Any[a === Bottom ? Any : a for a in argtypes] # assuming the previous error is fixed, keep analysis going
    @invoke abstract_call(interp::AbstractInterpreter, ea::Union{Nothing,Vector{Any}}, argtypes::Vector{Any},
                          sv::InferenceState, max_methods::Int)
end

and keep analysis going and collect as much errors as possible even when we know the actual execution terminates earlier.

I agree with this interface would be weird from optimization perspective, though.

/cc @vtjnash because you added commit to revert these interfaces in #39606. Could I hear your ideas on this ?

[Keno]

Hmm, I thought you wanted to use these to early-out in useless analysis rather than to continue into unreachable code-paths. I think doing the latter can be pretty tricky.

[vtjnash]

Those aren't the same conditions though. If you did want to bail early, it should be using the Union{} lattice element (otherwise we may exit before the lattice has finished converging). If you want to bail late, that decision doesn't happen here either (if later tfuncs are correct, they should also return Union{} in this case, but it would be a wild mess add this check everywhere). That decision happens in typeinf_local, where we decide to stop forwarding information to the next statement rather than, say, attempting to set changes = fill!(similar(changes), VarState(Union{}, true)) so that we reach each subsequent statement (Though most will also error. Since this is dead/unreachable code, that is what we want, to avoid poisoning the actual inference results at the next phi node join point.)

I agree you could replace Union{} with Any (not here, but when they are accessed from the Slot), but that might quite quickly confuse the convergence algorithm and drive many more function towards being uninferrable. To avoid that, I think you'd realistically need to run inference repeatedly, "fixing" one error at a time from the outside driver.

[vtjnash]

In general, I strongly dislike parametrization just for the sake of parameterization, since it bloats the compiler and makes it harder to follow the logical flow.

[aviatesk]

Thanks for your responses.
Fair. As you pointed out my idea will be really complicated otherwise it will ruin inference results and convergence.
Even when I try that in the future, I think I will end up overloading the entire body of typeinf_local anyway, so honestly I'm not too stick with bail_out_local/statement interface and I'm okay with #39606.

In general, I strongly dislike parametrization just for the sake of parameterization, since it bloats the compiler and makes it harder to follow the logical flow.

Yeah, that's what I was worried about. Are you okay with bail_out_call/bail_out_apply ? I'm fairly sure they won't destroy the convergence nor inference results anyway, but still might look weird in term of the implementation of NativeInterpreter. Still they are very useful for the implementation of JETInterpreter.

[vtjnash]

Those seem less bad, since it is probably just a performance optimization. There's also possibly not really any other correct behavior there to put in the overload either? We already computed the final call graph (info) and return type (Any), and no further analysis work should alter those. Thus also where we see how the optimizer is designed to run as a separated pass, and consumes both of those bits of information later and avoids being dependent upon the precise way it arrived at the answer.