Equivalent CodeInstance replacement does not update / consider edges

[topolarity]

Noticed this while working on TypedCallable.

We use this equivalence predicate to check whether an invokee CodeInstance can be swapped out for another in the cache before codegen:

julia/Compiler/src/typeinfer.jl

Lines 1723 to 1731 in f89b539

	cached = ccall(:jl_get_ci_equiv, Any, (Any, UInt), callee, get_inference_world(workqueue.interp))::CodeInstance
	if cached === callee
	# make sure callee is gc-rooted and cached, as required by jl_add_codeinsts_to_jit
	code_cache(workqueue.interp)[mi] = callee
	else
	# use an existing CI from the cache, if there is available one that is compatible
	callee === ci && (ci = cached)
	callee = cached
	end

and then to support invoking the cached "equivalent" CI, our AOT / JIT codegen has a very similar notion of "equivalence" to choose a target CI:

julia/src/jitlayers.cpp

Lines 2331 to 2334 in f89b539

	return jl_atomic_load_relaxed(&CI2->min_world) <= MinWorld &&
	jl_atomic_load_relaxed(&CI2->max_world) >= MaxWorld &&
	jl_egal(CI2->def, Def) && jl_egal(CI2->owner, Owner) &&
	jl_egal(CI2->rettype, RetType);

but notably this does not replace the CodeInstance in the :invoke or .edges of the CI.

IPO-safety guarantees us that this replacement is sound even if the exct, ipo_purity_bits, etc. from the new CodeInstance are wide compared to the CI originally inferred against, but this guarantee only applies point-wise in world age - it requires that we continue to respect the world / invalidation bounds for the invoked CI. My concern is that if later the "equivalent" CI is invalidated but your original CI in your .edges was not, it seems that this can become a stale invalidation bug.

Even before then, it appears that the world bounds for the produced CI can be invalid if the "equivalent" CI has a more narrow world range.

If reachable this bug is quite hard to hit in practice, since it relies on two different runs for the same MethodInstance yielding appreciably different inference results, which for the time being ~usually only happens due to cache effects, etc.

I put Claude 🤖 on reproducing, but we've gotten sidetracked after discovering other missing edges.

[vtjnash]

None of those sound like bugs, but are in fact the definition of what it means to be an invoke edge (see https://hackmd.io/sjPig55kS4a5XNWC6HmKSg#Edges for more details on the implications). As your quoted code shows:

Even before then, it appears that the world bounds for the produced CI can be invalid if the "equivalent" CI has a more narrow world range

is checked as part of the definition of equiv

[vtjnash]

two different runs for the same MethodInstance yielding appreciably different inference results,

Ignoring the effect bits in particular makes this arguably somewhat ~analogous to our if @generated soundness troubles.

It is essentially the same situation, but without the soundness troubles--by definition, when inference produces appreciably different results, the more true answer is the intersection of those.

[topolarity]

one of those sound like bugs, but are in fact the definition of what it means to be an invoke edge

Yeah you're right. On later reflection, I realized that my part (1) is fine by our notion of IPO-safety

That part is edited away now.

is checked as part of the definition of equiv

The codegen check does, but the compile! version only checks against the target world

None of those sound like bugs

I beg to differ 😉

module Foo # in a pkgimage
export W, foo
struct W{T}; x::T; end
@noinline target() = Core.compilerbarrier(:const, 1)
@noinline foo(::W{W{W{W{W{W{Int8}}}}}})  = target()
@noinline foo(::W{W{W{W{W{W{Int16}}}}}}) = target()
@noinline foo(::W{W{W{W{W{W{Int32}}}}}}) = target()
@noinline foo(::W{W{W{W{W{W{Int64}}}}}}) = target()
@noinline foo(::W{W{W{W{W{W{Bool}}}}}})  = target()
@noinline foo(w::W)::Int = foo(W(w))
precompile(foo, (W{Int},))     # serialize a widened CI w/ weak edges
end # module Foo

# mwe.jl
using Foo
for T in (W{W{W{W{W{W{Int}}}}}}, W{W{W{W{W{Int}}}}}, W{W{W{W{Int}}}}, W{W{W{Int}}}, W{W{Int}})
    precompile(foo, (T,))
end
precompile(foo, (W{Int},))

before = foo(W(1))
@eval Foo target() = 99 # invalidates the CI we are executing, but not the one in our edges
(after, expected) = (foo(W(1)), Foo.target())
@show before after expected

$ julia +nightly --project=Foo --pkgimages=no mwe.jl
before = 1
after = 1 # oops
expected = 99

[vtjnash]

Ah, gotcha, yeah, the wildcard world (typemax(UInt64)) is tricky there since it should have a dynamically unbounded set of edges, so we have to think about how to be more careful when we track that