inference: reinfer and track missing code for inlining

[vtjnash]

When code is potentially needed for inlining, but missing for any reason, be sure to regenerate it during inference with the correct ci_meets_requirement flags (SOURCE_MODE_GET_SOURCE instead of NOT_REQUIRED) so it is prepared for the optimizer if needed.

This was supposed to be the correct fix for someone else's bug with missing inlining when expected, but I don't remember what bug it was anymore.

julia> @atomic Base.method_instance(promote, (Float64,Int)).cache.inferred = 0x22
0x22

julia> @code_typed 1.0+1
CodeInfo(
    @ promotion.jl:433 within `+`
1 ─ %1 =    invoke Base.promote(x::Float64, y::Int64)::Tuple{Float64, Float64}
│   %2 =   builtin Core.getfield(%1, 1)::Float64
│   %3 =   builtin Core.getfield(%1, 2)::Float64
│   @ promotion.jl:433 within `+` @ float.jl:492
│   %4 = intrinsic Base.add_float(%2, %3)::Float64
└──      return %4
) => Float64

Fixes #59846

[JeffBezanson]

I'm aware of a couple inlining issues, e.g. #58906, and

@code_typed ((a, b) -> @inline(Base.setindex(a, b, :next)))((next = zero(UInt32), prev = zero(UInt32)), 2)

and

julia> myconvert3(x; T::Type{V}=Int) where {V} = convert(V,x)
julia> f(x) = myconvert3(x;T=Int)
julia> code_typed(f, (Float64,))
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 =    invoke Core.kwcall((T = Int64,)::@NamedTuple{T::DataType}, Main.myconvert3::typeof(myconvert3), x::Float64)::Any
└──      return %1
) => Any

(though these might not all be strictly just inlining). This does not fix any of them, but we should try to find an example.

[vtjnash]

No, those are the sort of issues. This is more related to precompile files causing performance to suffer later. (and that first example is already fixed on master, I think by c05b68f)

[aviatesk]

I think this definition should only be used for NativeInterpreter, and for AbstractInterpreter it should be defined to perform a lookup to code_cache by default?
The current test failure should also be fixed by this.

[vtjnash]

Yes, using codegen_cache felt like a hack I added just for getting the PR started. With your refactoring, I think we should not need this anymore for any interpreter (since the code should now be in the regular local cache)? I'm not really sure there is a point to having separate codegen_cache and local_cache--seems like we can and should just merge those, so that everything done in this AbstractInterpreter is now predictably in local_cache; and optionally also in global_cache.

[vtjnash]

I believe I finally tracked down the last failing test to be an IRInterp problem. While the normal interpreter prepares code for successful inlining with update stmt info, the IRInterp appears that it may not. So since it appears that disabling IRInterp bypasses that bug, it should be possible to make sure IRInterp is doing the correct work in abstract_call to propagate the call result info, so that when inlining processes the result of IRInterp, it gets the same result as if it was processing the result of the normal interp.

[vtjnash]

I finally tracked it down even more fully. It is a known implementation problem with IRInterp that it degrades code optimization quality when used, which has had a TODO in the code from #48913, but it was previously hidden from tests by the combination of luck, this bug, #58183, and that we have incomplete test coverage of when exactly inlining must occur. I put a hack in place to ensure IRInterp is used no more nor less after this PR than before, so that it cannot cause any new regressions than already existed.

[aviatesk]

Thanks for looking into the test failures.

I put a hack in place to ensure IRInterp is used no more nor less after this PR than before, so that it cannot cause any new regressions than already existed.

Could you elaborate what specific fixes you made for this?

I thought most of the tests newly marked as broken were because the ci_get_source implementation doesn't support external AbstractInterpreter.
I'm going to make a fix for this and address the tests marked broken in this PR today.

[vtjnash]

Seems good. It is a little awkward intermediate state, since once this PR is done, we should deprecate ci_get_source (inference is supposed to provide it in the InferenceResult, but right now there are about a dozen different ways to get approximately the same thing)

Could you elaborate what specific fixes you made for this?

Prohibits IRInterp unless the method body is inlineable. That aligns with the old implementation, before this PR fixed the problems with getting code and caused the worse code generated by IRInterp to be caught by an existing test.

[vtjnash]

Looks like the other current test failure just needs to be relaxed somewhat on ordering to just make sure flbi is invalidating useflbi:

diff --git a/test/precompile.jl b/test/precompile.jl
index 04ec1b2e007..73dbd7b99c6 100644
--- a/test/precompile.jl
+++ b/test/precompile.jl
@@ -1124,9 +1124,9 @@ precompile_test_harness("code caching") do dir
             idxv = findnext(==("verify_methods"), invalidations, idxv+1)
         end
         @test invalidations[idxv-1].def.def.name === :flbi
-        idxv = findnext(==("verify_methods"), invalidations, idxv+1)
-        @show invalidations
-        @test invalidations[idxv-1].def.def.name === :useflbi
+        idxv = findnext(==(invalidations[idxv-1]), invalidations, idxv+1)
+        @test invalidations[idxv-1] == "verify_methods"
+        @test invalidations[idxv-2].def.def.name === :useflbi
 
         m = only(methods(MB.map_nbits))
         @test !hasvalid(m.specializations::Core.MethodInstance, world+1) # insert_backedges invalidations also trigger their backedges

[aviatesk]

Seems good. It is a little awkward intermediate state, since once this PR is done, we should deprecate ci_get_source (inference is supposed to provide it in the InferenceResult, but right now there are about a dozen different ways to get approximately the same thing)

I agree.
How far should we go with this PR?
Personally, I would like to update JET/Cthulhu to work with this PR as well, but at least for Cthulhu, it would be better to finish Cedric's PR first, and waiting for that might unnecessarily block this PR.

[DilumAluthge]

Ah. In that case, can I temporarily disable the flaky test?

#59879