Do not round-trip uncached inference results through IRCode

[Keno]

There's generally three reasons inference results end up uncached:

1. They come from typeinf_ext
2. We discover some validity limitation (generally due to recursion)
3. They are used for constant propagation

Currently, we convert all such inference results back to CodeInfo, in case they come from 1. However, for inference results of kind 3, the only thing we ever do with them is turn them back into IRCode for inlining. This round-tripping through IRCode is quite wasteful. Stop doing that. This PR is the minimal change to accomplish that by marking those inference results that actually need to be converted back (for case 1). This probably needs some tweaking for external AbstractInterpreters, but let's make sure this works and has the right performance first.

[Keno]

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

[nanosoldier]

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

[Keno]

Your benchmark job has completed - possible performance regressions were detected

Well, that's disappointing. Will look into it in the morning.

[aviatesk]

typeinf_ext_toplevel and typeinf_frame also want to have (frame::InferenceState).src, so we need to mark their results as well. The typeinf_edge for callsite-inlined frame also needs special treatment.

So we need something like:

diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl
index 1d119660d3..443b184803 100644
--- a/base/compiler/typeinfer.jl
+++ b/base/compiler/typeinfer.jl
@@ -932,6 +932,9 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
         # completely new
         lock_mi_inference(interp, mi)
         result = InferenceResult(mi)
+        if cache === :local
+            result.must_be_codeinf = true # TODO directly keep `opt.ir` for this case
+        end
         frame = InferenceState(result, cache, interp) # always use the cache for edge targets
         if frame === nothing
             # can't get the source for this, so we know nothing
@@ -1005,6 +1008,7 @@ function typeinf_frame(interp::AbstractInterpreter, method::Method, @nospecializ
     mi = specialize_method(method, atype, sparams)::MethodInstance
     ccall(:jl_typeinf_timing_begin, Cvoid, ())
     result = InferenceResult(mi)
+    result.must_be_codeinf = true
     frame = InferenceState(result, run_optimizer ? :global : :no, interp)
     frame === nothing && return nothing
     typeinf(interp, frame)
@@ -1063,8 +1067,9 @@ function typeinf_ext(interp::AbstractInterpreter, mi::MethodInstance)
         return retrieve_code_info(mi)
     end
     lock_mi_inference(interp, mi)
-    frame = InferenceState(InferenceResult(mi), #=cache=#:global, interp)
-    frame.result.must_be_codeinf = true
+    result = InferenceResult(mi)
+    result.must_be_codeinf = true
+    frame = InferenceState(result, #=cache=#:global, interp)
     frame === nothing && return nothing
     typeinf(interp, frame)
     ccall(:jl_typeinf_timing_end, Cvoid, ())
@@ -1107,6 +1112,7 @@ function typeinf_ext_toplevel(interp::AbstractInterpreter, linfo::MethodInstance
             ccall(:jl_typeinf_timing_begin, Cvoid, ())
             if !src.inferred
                 result = InferenceResult(linfo)
+                result.must_be_codeinf = true
                 frame = InferenceState(result, src, #=cache=#:global, interp)
                 typeinf(interp, frame)
                 @assert frame.inferred # TODO: deal with this better

[vtjnash]

Is there ever a reason we would want this field to be a CodeInfo? Seems like only the global cache needs that (preparing for codegen and other outside consumers), but that inlining would always be happier to get the IRCode directly if we had it

[Keno]

Is there ever a reason we would want this field to be a CodeInfo? Seems like only the global cache needs that (preparing for codegen and other outside consumers), but that inlining would always be happier to get the IRCode directly if we had it

typeinf_ext sometimes wants codeinfos back that it doesn't put in the global cache IIRC.

[Keno]

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

[nanosoldier]

Your job failed.

[Keno]

Ugh, cfg_simplify! has too many bugs and compact!(ir, true) is too slow. I don't want to track this down right now. I'll just switch the default to what this used to be, so I can play with this in my external AbstractInterpreter setup and worry about the base case later.

[Keno]

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

[nanosoldier]

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