reenable global ref lookup optimization

benchmark/benchmarks.jl

@@ -65,4 +65,44 @@ SUITE["ccall ptr"] = @benchmarkable @interpret ccall_ptr(ptr, 1, 5)
 function powf(a, b)
     ccall(("powf", Base.Math.libm), Float32, (Float32,Float32), a, b)
 end
-SUITE["ccall library"] = @benchmarkable @interpret powf(2, 3)
+SUITE["ccall library"] = @benchmarkable @interpret powf(2, 3)
+
+# Global reads in a tight loop: `const` globals can be folded at framecode construction,
+# non-`const` globals must be looked up on every execution
+module BenchGlobals
+const CONSTGLOBAL = 1
+NONCONST = 1
+end
+
+function readconst(n)
+    s = 0
+    for i in 1:n
+        s += BenchGlobals.CONSTGLOBAL
+    end
+    return s
+end
+SUITE["const global read 10_000"] = @benchmarkable @interpret readconst(10_000)
+
+function readnonconst(n)
+    s = 0
+    for i in 1:n
+        s += BenchGlobals.NONCONST
+    end
+    return s
+end
+SUITE["nonconst global read 10_000"] = @benchmarkable @interpret readnonconst(10_000)
+
+# Calls resolved through globals: every call statement's callee is a `GlobalRef`
+function callglobals(x)
+    s = zero(x)
+    for i in 1:1_000
+        s += sin(x) + cos(x) + abs(x) + min(x, one(x))
+    end
+    return s
+end
+SUITE["global callees 1_000"] = @benchmarkable @interpret callglobals(0.5)
+
+# Frame creation: uncached measures the framecode build (including the `optimize!` pass),
+# cached measures the per-call frame setup for an already-built framecode
+SUITE["frame creation uncached"] = @benchmarkable JuliaInterpreter.enter_call(callglobals, 0.5) setup=(JuliaInterpreter.clear_caches()) evals=1
+SUITE["frame creation cached"] = @benchmarkable JuliaInterpreter.enter_call(callglobals, 0.5)

src/commands.jl

@@ -490,15 +490,13 @@ end
 maybe_step_through_kwprep!(frame::Frame, istoplevel::Bool=false) =
     maybe_step_through_kwprep!(RecursiveInterpreter(), frame, istoplevel)
 
-@static if isbindingresolved_deprecated
-    function is_empty_namedtuple(stmt)
-        isexpr(stmt, :call) && length(stmt.args) == 1 || return false
-        arg1 = stmt.args[1]
-        isa(arg1, GlobalRef) || return false
-        return arg1.name === :NamedTuple
-    end
-else
-    is_empty_namedtuple(stmt) = isexpr(stmt, :call) && is_quoted_type(stmt.args[1], :NamedTuple) && length(stmt.args) == 1
+# The `NamedTuple` callee is a `QuoteNode` when `optimize!` folded the const (method scope),
+# or a `GlobalRef` when it didn't (toplevel scope on 1.12+); accept both forms.
+function is_empty_namedtuple(stmt)
+    isexpr(stmt, :call) && length(stmt.args) == 1 || return false
+    arg1 = stmt.args[1]
+    is_quoted_type(arg1, :NamedTuple) && return true
+    return isa(arg1, GlobalRef) && arg1.name === :NamedTuple
 end
 
 """
@@ -699,11 +697,13 @@ function debug_command(interp::Interpreter, frame::Frame, cmd::Symbol, rootistop
                 # On Julia 1.12 a method body may start with bare global loads
                 # (e.g. the `+` of `f(x) = g(x) + 1`); pausing there shows the
                 # user an internal-looking statement that is not even the next
-                # call. Advance to the first call or return, like frame entry
-                # does. Keyword/closure bodies (gensym `#` names) keep their
-                # exact entry point.
+                # call. `optimize!` folds `const` globals to `QuoteNode`s, so the
+                # leading load is either a `GlobalRef` or a `QuoteNode`. Advance
+                # to the first call or return, like frame entry does.
+                # Keyword/closure bodies (gensym `#` names) keep their exact entry point.
                 scope = scopeof(newframe)
-                normalize_entry = pc_expr(newframe) isa GlobalRef &&
+                entrystmt = pc_expr(newframe)
+                normalize_entry = (entrystmt isa GlobalRef || entrystmt isa QuoteNode) &&
                     !(scope isa Method && startswith(string(scope.name), "#"))
                 if !is_si && normalize_entry
                     pc = maybe_next_until!(interp, newframe, istoplevel) do fr::Frame

src/optimize.jl

@@ -10,11 +10,29 @@ const compiled_calls = Dict{Any,Any}()
 function record_world_dep!(world_deps::Union{Nothing,Vector{BindingPartition}}, world::UInt, gr::GlobalRef)
     @static if isbindingresolved_deprecated
         world_deps === nothing && return nothing
-        push!(world_deps, Base.lookup_binding_partition(world, gr))
+        # KNOWN HOLE: only the named binding's partition is recorded. For an explicitly
+        # imported binding (`import M: x` / `using M: x`) that partition delegates to the
+        # source binding and survives a rebinding of the source const — only the source
+        # partition (where the value lives) gets split. `lookup_global_ref` therefore never
+        # folds such bindings, but the compiled `ccall`/`llvmcall` wrapper paths still bake
+        # values reached through them and would miss the rebinding (see the
+        # "imported const invalidation" `@test_broken`). Whether importer partitions should
+        # be split upstream, or the delegation chain recorded here, is under discussion.
+        record_world_dep!(world_deps, Base.lookup_binding_partition(world, gr))
     end
     return nothing
 end
 
+@static if isbindingresolved_deprecated
+function record_world_dep!(world_deps::Union{Nothing,Vector{BindingPartition}}, bpart::BindingPartition)
+    world_deps === nothing && return nothing
+    # The same binding is typically referenced many times within one method; store each
+    # partition once so `framecode_valid_world` stays cheap.
+    any(p -> p === bpart, world_deps) || push!(world_deps, bpart)
+    return nothing
+end
+end
+
 # Record every `GlobalRef` reachable in `arg` (recursing through `Expr`s, but not chasing
 # `SSAValue`s). Used when a whole expression's value is baked at framecode-build time, e.g. a
 # `(name, lib)` tuple evaluated for a compiled `ccall` wrapper or the feeder statements of an
@@ -79,32 +97,55 @@ function smallest_ref(stmts, arg, idmin)
     return idmin
 end
 
-function lookup_global_ref(a::GlobalRef, world::UInt)
-    isbindingresolved_deprecated && return a   # TODO: reenable this optimization once we can invalidate Frames
-    if Base.isbindingresolved(a.mod, a.name) &&
-        (invoke_in_world(world, isdefinedglobal, a.mod, a.name)) &&
-        (invoke_in_world(world, isconst, a.mod, a.name))
-        return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name))
+function lookup_global_ref(a::GlobalRef, world::UInt,
+                           world_deps::Union{Nothing,Vector{BindingPartition}}=nothing)
+    @static if isbindingresolved_deprecated
+        # On 1.12+ a `const` can be rebound, making the folded value world-dependent. Folding
+        # is therefore allowed only when the caller supplies `world_deps`: the binding
+        # partition recorded there lets `framecode_valid_world` reject the cached framecode
+        # for worlds in which the binding was redefined.
+        world_deps === nothing && return a
+        bpart = Base.lookup_binding_partition(world, a)
+        # Fold only when the partition itself holds the constant value (a directly defined
+        # or implicit-`using` const): then this partition must be split for the value to
+        # change, so recording it makes the invalidation exact. Explicitly imported bindings
+        # (`import M: x` / `using M: x`) delegate to the source binding and their partition
+        # survives a rebinding of the source const, so a folded value could go stale
+        # undetected — leave those as `GlobalRef`s (resolved per execution in the frame's
+        # world, which is always correct).
+        if Base.is_defined_const_binding(Base.binding_kind(bpart))
+            record_world_dep!(world_deps, bpart)
+            return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name))
+        end
+        return a
+    else
+        if Base.isbindingresolved(a.mod, a.name) &&
+            (invoke_in_world(world, isdefinedglobal, a.mod, a.name)) &&
+            (invoke_in_world(world, isconst, a.mod, a.name))
+            return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name))
+        end
+        return a
     end
-    return a
 end
 
-function lookup_global_refs!(ex::Expr, world::UInt)
+function lookup_global_refs!(ex::Expr, world::UInt,
+                             world_deps::Union{Nothing,Vector{BindingPartition}}=nothing)
     if isexpr(ex, (:isdefined, :thunk, :toplevel, :method, :global, :const, :globaldecl))
         return nothing
     end
     for (i, a) in enumerate(ex.args)
         ex.head === :(=) && i == 1 && continue # Don't look up globalrefs on the LHS of an assignment (issue #98)
         if isa(a, GlobalRef)
-            ex.args[i] = lookup_global_ref(a, world)
+            ex.args[i] = lookup_global_ref(a, world, world_deps)
         elseif isa(a, Expr)
-            lookup_global_refs!(a, world)
+            lookup_global_refs!(a, world, world_deps)
         end
     end
     return nothing
 end
 
-function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt)
+function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt,
+                              world_deps::Union{Nothing,Vector{BindingPartition}}=nothing)
     isexpr(a, :call) || return a
     length(a.args) == 3 || return a
     arg1 = lookup_stmt(code, a.args[1], world)
@@ -113,17 +154,16 @@ function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt)
     arg2 isa Module || return a
     arg3 = lookup_stmt(code, a.args[3], world)
     arg3 isa Symbol || return a
-    return lookup_global_ref(GlobalRef(arg2, arg3), world)
+    return lookup_global_ref(GlobalRef(arg2, arg3), world, world_deps)
 end
 
 # HACK This isn't optimization really, but necessary to bypass llvmcall and foreigncall
 # TODO This "optimization" should be refactored into a "minimum compilation" necessary to
 # execute `llvmcall` and `foreigncall` and pure optimizations on the lowered code representation.
-# On Julia 1.12+ the GlobalRef -> QuoteNode folding is disabled (`lookup_global_ref` returns the
-# ref unchanged) because a redefinable `const` makes the folded value world-dependent; values
-# that *must* be resolved at build time (library names and llvmcall ingredients baked into the
-# compiled wrappers below) record their bindings in `world_deps` so the framecode can be
-# invalidated when a binding is redefined.
+# On Julia 1.12+ a redefinable `const` makes a folded value world-dependent, so every value
+# resolved at build time — folded `const` globals as well as library names and llvmcall
+# ingredients baked into the compiled wrappers below — records its binding in `world_deps`,
+# and `framecode_valid_world` rejects the cached framecode once any of them is redefined.
 
 """
     optimize!(code::CodeInfo, scope, world::UInt) -> code, methodtables, world_deps
@@ -142,24 +182,29 @@ function optimize!(code::CodeInfo, scope, world::UInt)
     sparams = scope isa Method ? sparam_syms(scope) : Symbol[]
     replace_coretypes!(code)
 
-    # Binding partitions of globals whose values get baked into this framecode (compiled
-    # `ccall`/`llvmcall` wrappers); recorded so a cached `FrameCode` can be invalidated once any
-    # baked value goes stale (see `FrameCode.world_deps`).
+    # Binding partitions of globals whose values get baked into this framecode (folded `const`
+    # globals and compiled `ccall`/`llvmcall` wrappers); recorded so a cached `FrameCode` can be
+    # invalidated once any baked value goes stale (see `FrameCode.world_deps`).
     world_deps = BindingPartition[]
+    # On 1.12+, fold `const` globals only for method scope: the framecode cache is guarded by
+    # `framecode_valid_world`, and a method frame's world is fixed at construction. Toplevel
+    # frames advance `frame.world` mid-execution (see `step_toplevel!`), so a value folded in
+    # the build world could go stale within the frame; leave their `GlobalRef`s unresolved.
+    fold_deps = scope isa Method ? world_deps : nothing
     # TODO: because of builtins.jl, for CodeInfos like
     #   %1 = Core.apply_type
     #   %2 = (%1)(args...)
     # it would be best to *not* resolve the GlobalRef at %1
     ## Replace GlobalRefs with QuoteNodes
     for (i, stmt) in enumerate(code.code)
         if isa(stmt, GlobalRef)
-            code.code[i] = lookup_global_ref(stmt, world)
+            code.code[i] = lookup_global_ref(stmt, world, fold_deps)
         elseif isa(stmt, Expr)
             if stmt.head === :call && stmt.args[1] === :cglobal  # cglobal requires literals
                 continue
             else
-                lookup_global_refs!(stmt, world)
-                code.code[i] = lookup_getproperties(code.code, stmt, world)
+                lookup_global_refs!(stmt, world, fold_deps)
+                code.code[i] = lookup_getproperties(code.code, stmt, world, fold_deps)
             end
         end
     end

src/types.jl

@@ -189,11 +189,11 @@ function is_breakpoint_expr(ex::Expr)
     return isa(q, QuoteNode) && q.value === :__BREAKPOINT_MARKER__
 end
 
-@static if isbindingresolved_deprecated
-    is_breakpoint_marker(stmt) = is_global_ref(stmt, JuliaInterpreter, :__BREAK_POINT_MARKER__)
-else
-    is_breakpoint_marker(stmt) = stmt === __BREAK_POINT_MARKER__
-end
+# `@bp` lowers to a `GlobalRef` of the `__BREAK_POINT_MARKER__` const. `optimize!` folds it
+# to its value in method scope (unwrapped from the `QuoteNode` by `lookup_stmt`), while
+# toplevel or unoptimized code keeps the `GlobalRef`, so accept both forms.
+is_breakpoint_marker(@nospecialize(stmt)) =
+    stmt === __BREAK_POINT_MARKER__ || is_global_ref(stmt, JuliaInterpreter, :__BREAK_POINT_MARKER__)
 
 @static if VERSION ≥ v"1.12.0-DEV.173"
 function pushuniquefiles!(unique_files::Set{Symbol}, lt::Core.DebugInfo)

test/interpret.jl

@@ -1344,14 +1344,16 @@ end
         @test !isempty(fcchain.world_deps)   # the `Base.Math.libm` getproperty-chain resolution
 
         # Rebinding the library const (e.g. after dlclosing one library and dlopening another)
-        # invalidates the framecode; the rebuild resolves the new value, so the call fails on the
-        # bogus path instead of silently calling into the stale library.
+        # invalidates the framecode; a rebuild in a world that sees the new value resolves it, so
+        # the call fails on the bogus path instead of silently calling into the stale library.
+        # (In worlds predating the rebinding — like this testset's task world — the old value is
+        # still the correct resolution, matching compiled-code semantics.)
         Core.eval(WrapperDepTest, :(const MYLIB = "/nonexistent_library_path"))
         w2 = Base.get_world_counter()
         @test !JuliaInterpreter.framecode_valid_world(fc, w2)
         fc2, _ = JuliaInterpreter.prepare_framecode(mlib, Tuple{typeof(WrapperDepTest.powf_constlib), Float32, Float32}; world=w2)
         @test fc2 !== fc
-        @test_throws "nonexistent_library_path" @interpret(WrapperDepTest.powf_constlib(2f0, 3f0))
+        @test_throws "nonexistent_library_path" @interpret world=w2 WrapperDepTest.powf_constlib(2f0, 3f0)
         # An unrelated rebinding must not invalidate the chain-library framecode.
         @test JuliaInterpreter.framecode_valid_world(fcchain, w2)
 
@@ -1371,6 +1373,53 @@ end
     end
 end
 
+module ImportedConstSource
+    const XI = 1
+    const XU = 1
+    const LIB = Base.Math.libm
+end
+module ImportedConstConsumer
+    import ..ImportedConstSource: XI, LIB
+    using ..ImportedConstSource: XU
+    fi() = XI
+    fu() = XU
+    powf_importedlib(a, b) = ccall(("powf", LIB), Float32, (Float32, Float32), a, b)
+end
+
+@static if JuliaInterpreter.isbindingresolved_deprecated
+@testset "imported const invalidation" begin
+    # Explicitly imported consts (`import M: x` / `using M: x`) are never folded: the
+    # importing module's binding partition delegates to the source binding and is NOT split
+    # when the source const is rebound (only the source partition is), so a folded value
+    # could go stale without invalidating the framecode. The `GlobalRef` is left in place
+    # and resolved per execution in the frame's world, which is correct in every world.
+    for (f, src_name) in ((ImportedConstConsumer.fi, :XI), (ImportedConstConsumer.fu, :XU))
+        m = only(methods(f))
+        w = Base.get_world_counter()
+        fc, _ = JuliaInterpreter.prepare_framecode(m, Tuple{typeof(f)}; world=w)
+        @test any(x -> isa(x, GlobalRef), fc.src.code)   # not folded
+        Core.eval(ImportedConstSource, :(const $src_name = 2))
+        w2 = Base.get_world_counter()
+        @test JuliaInterpreter.framecode_valid_world(fc, w2)  # nothing baked, still valid
+        @test (@interpret world=w2 f()) == 2  # matches native execution in w2
+        @test (@interpret world=w f()) == 1   # pre-rebinding world sees the old value
+    end
+
+    # KNOWN HOLE: the compiled-ccall wrapper path *bakes* values at build time regardless of
+    # binding kind, and `record_world_dep!` records only the importing partition, which
+    # survives the source rebinding — so the framecode wrongly stays valid. Whether importer
+    # partitions should be split upstream, or the delegation chain recorded in
+    # `record_world_dep!`, is still under discussion.
+    mlib = only(methods(ImportedConstConsumer.powf_importedlib))
+    w = Base.get_world_counter()
+    @test (@interpret world=w ImportedConstConsumer.powf_importedlib(2f0, 3f0)) == 8f0
+    fc, _ = JuliaInterpreter.prepare_framecode(mlib, Tuple{typeof(ImportedConstConsumer.powf_importedlib), Float32, Float32}; world=w)
+    Core.eval(ImportedConstSource, :(const LIB = "/nonexistent_library_path"))
+    w2 = Base.get_world_counter()
+    @test_broken !JuliaInterpreter.framecode_valid_world(fc, w2)
+end
+end # @static if
+
 @testset "Empty varargs are visible to locals" begin
     empty_vararg(x...) = x
     fr = JuliaInterpreter.enter_call(empty_vararg)