perform inference using optimizer-derived type information

[aviatesk]

In certain cases, the optimizer can introduce new type information. This is particularly evident in SROA, where load forwarding can reveal type information that was not visible during abstract interpretation. In such cases, re-running abstract interpretation using this new type information can be highly valuable, however, currently, this only occurs when semi-concrete interpretation happens to be triggered.

This commit introduces a new "post-optimization inference" phase at the end of the optimizer pipeline. When the optimizer derives new type information, this phase performs IR abstract interpretation to further optimize the IR.

Such cases are especially common in patterns involving captured
variables, as discussed in #15276. By combining the
"post-optimization inference" implemented in this commit with EA-based
load forwarding, it is anticipated that the issue described in
#15276 can largely be resolved.

---

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

[nanosoldier]

Your job failed.

[oscardssmith]

does this ensure that we don't end up inferring non-IPO-safe things in an IPO context?

[aviatesk]

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

[nanosoldier]

Your job failed.

[aviatesk]

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

[nanosoldier]

Your job failed.

[aviatesk]

@nanosoldier runbenchmarks(["inference", "allinference", "Base.init_stdio(::Ptr{Cvoid})"], vs=":master")

[aviatesk]

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

[nanosoldier]

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

[aviatesk]

@nanosoldier runbenchmarks(!"scalar", vs=":master")

[nanosoldier]

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

[aviatesk]

@nanosoldier runbenchmarks(!"scalar", vs=":master")

[aviatesk]

does this ensure that we don't end up inferring non-IPO-safe things in an IPO context?

In the current implementation of the optimizer, non-IPO-safe optimizations are not performed. So I understand that performing re-inference on such IPO-safe IRCode does not lead to IPO-validity issues.

[nanosoldier]

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

[aviatesk]

@nanosoldier runbenchmarks("collection" || "simd" || "sort", vs=":master")

[nanosoldier]

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

[aviatesk]

I have confirmed that there are no test failures or performance regressions. This PR is ready to be merged.

[vtjnash]

I think the main question for review is whether this PR makes codegen unsound. Right here, we publish the ABI that will be used by this CodeInstance, for use by the optimizer in creating invoke edges. Any future changes to that info can result in codegen becoming unsound (as well as thread-unsafe).

[vtjnash]

Changing any non-atomic fields here (after cache_result) causes codegen to become unsound. I am uncertain how we would preserve the expectation that the optimizer only uses CodeInstances found in the cache, while also delaying publishing them into the cache until after this point in the code. I don't believe it to be impossible to change that (since we are holding the "engine lock" on this until after the optimizer finishes, so we can just predict which CodeInstance will get added to the cache and which are only for local use), but just tricky to figure out how to best correctly adapt the cache to this PR.

[aviatesk]

@vtjnash Thank you for the review.

If I understand correctly, this PR has two main issues:

1. Updating rettype or other (non-atomic) information defining the CodeInstance's ABI after cache_result!.
2. Performing re-inference during optimization breaks the assumption that the optimizer operates with the global cache stabilized

Regarding (1), it seems that only updating the inferred field may be the viable solution? (Honestly, even if the ABI changes between abstract interpretation and the optimizer, I don’t think it would cause significant issues in most cases, but I understand your concern that introducing a fundamental error could create future risks.)

As for (2), I don’t understand why the optimizer must rely on a stabilized cache. Is the concern that optimization itself might no longer guarantee termination? Even so, it might be possible to resolve the issue by preventing re-inference from performing new inferences and restricting it to use only the results available in the global cache.

[vtjnash]

For (2), the main issue is just that we want the edges to normally come from the global cache, so that we don't accidentally make multiple copies of it and greatly slow down the whole system by accident. The main way we avoid this is with locks (e.g. the engine_lock) so that it can wait for in-progress work to finish at the necessary time, and a local cache to store any in-progress work.

So for both issues, we mainly just need to move the cache_result call until after optimization finishes, and then provide an additional intermediate "future global cache" cache that the optimizer can examine

[MasonProtter]

Interestingly, this doesn't seem to be able to catch cases where the boxed variable is the function itself (i.e. #53295)

julia> function outer()
           function inner()
               false && inner()
           end
           inner()
       end;

julia> @btime outer()
  14.758 ns (2 allocations: 32 bytes)
false

julia> @code_warntype outer()
MethodInstance for outer()
  from outer() @ Main REPL[13]:1
Arguments
  #self#::Core.Const(Main.outer)
Locals
  inner@_2::Core.Box
  inner@_3::Union{}
  inner@_4::Union{}
Body::Any
1 ─       (inner@_2 = Core.Box())
│   %2  = Main.:(var"#inner#8")::Core.Const(var"#inner#8")
│   %3  = inner@_2::Core.Box
│   %4  = %new(%2, %3)::var"#inner#8"
│   %5  = inner@_2::Core.Box
│         Core.setfield!(%5, :contents, %4)
│   %7  = inner@_2::Core.Box
│   %8  = Core.isdefined(%7, :contents)::Bool
└──       goto #3 if not %8
2 ─       goto #4
3 ─       Core.NewvarNode(:(inner@_4))
└──       inner@_4
4 ┄ %13 = inner@_2::Core.Box
│   %14 = Core.getfield(%13, :contents)::Any
│   %15 = (%14)()::Any
└──       return %15

even though this seems like it should be closely related to equivalent things with non-callables being boxed.