Allow irinterp to refine nothrow effect

[Keno]

This addresses a remaining todo in the irinterp code to allow it to compute whether its particular evaluation refined nothrow. As a result, we can re-enable it for a larger class of ir (we had previously disabled it to avoid regressing cases where regular constprop was able to prove a nothrow refinement, but irinterp was not).

[aviatesk]

IR_FLAG_EFFECT_FREE seems a bit risky here since we may prove :effect_free-ness in a presence of effect-ful statement, e.g.

julia> mutable struct SafeRef{T}
           x::T
       end

julia> Base.getindex(x::SafeRef) = x.x

julia> Base.setindex!(x::SafeRef, y) = x.x = y

julia> Base.infer_effects((String,String,)) do x, y
           r = SafeRef(x)
           r[] = y
           r[]
       end
(+c,+e,+n,+t,+s,+m)

So I guess we need to check is_removable_if_unused here?

[Keno]

I think you're probably right, though IR_FLAG_EFFECT_FREE was there before, so this might be an existing bug. Ideally we should have a testcase for this.

[aviatesk]

I attempted to create a test case to trigger an error because of this issue, but it seems very difficult to invent one. Something like the example below would be close (it actually crashes due to a different bug in our SROA though).

Base.@assume_effects :consistent :nothrow @inline function foo(a::Int, b)
    r = Ref(a)
    try
        setfield!(r, :x, b)
        nothing
    catch err
        return isa(b, String)
    end
end

[aviatesk]

xref: #48067

[aviatesk]

I'm fine with move this forward leaving the correctness issue for the future.

[Keno]

I looked into the broadcast test failure. The issue there turned out to be due to the --check-bounds=yes flag used during testing. The irinterp uses the IR cached in the sysimage, which has a mismatched check-bounds flag. If the sysimage is built with --check-bounds=yes, also the tests pass. That's perhaps a bit surprising, but what happens is that there are two bounds checks that throw different errors and the second of them captures an allocated ref, so if they're both there, only the first one counts, so the ref doesn't get allocated.

I don't think that's really a bug with this PR, I'd like to just allow this case in the test. Maybe in the future we can separate out the caches between check-bounds=yes and no.

[Keno]

@nanosoldier runtests()

[Keno]

#48110 + https://reviews.llvm.org/D140933 also makes the test work regardless by allowing LLVM to eliminate the boundscheck. However, I intend to decouple the two changes by allowing the failure here.

[aviatesk]

However, I intend to decouple the two changes by allowing the failure here.

Fine by me.

[nanosoldier]

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

[Keno]

A bunch of packages are complaining about lack of :splatnew support in irinterp, which should be easy enough to fix, and then looks like there's inference precision regressions in ChainRulesCore, ForwardDiff, FuncPipelines, JSOSolvers, PropertyDicts, SolverTools, Transits that need to be looked into.

[Keno]

ForwardDiff is the same failure as the broadcast test, so should be fixed by #48110.

[Keno]

Ok, I've gone through all of them and they should be fixed between #48144 and #48110 and the changes I made here. A general trend was that we should probably generally make methods that we give effect overrides @noinline, since otherwise irinterp does not have access to those overrides. Of course, we may also want to revisit using that information more during inlining to preserve it.

[aviatesk]

A general trend was that we should probably generally make methods that we give effect overrides @noinline, since otherwise irinterp does not have access to those overrides.

(Just to clarify) The problem here is that a :foldable method is no longer foldable during irinterp if it has been inlined. Especially, it needs to be :invoke call to be eligible for concrete_eval_invoke:

julia/base/compiler/ssair/irinterp.jl

Line 151 in 7d36744

if is_foldable(effects) && is_all_const_arg(argtypes, #=start=#1)

It might be a good idea to use the @noinline annotation on these methods because otherwise, we would need to perform irinterp on pre-inlining IR, which goes against the purpose of irinterp as a fast abstract interpretation on optimized IR.

[Keno]

(Just to clarify) The problem here is that a :foldable method is no longer foldable during irinterp if it has been inlined.

That's correct