From 06fc25272fe9342855534fed53e5d1f033df8f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Wed, 29 Jan 2025 05:29:17 -0500 Subject: [PATCH 1/8] Add proof of concept for caching runtime calls --- .../CompilerDevTools/src/CompilerDevTools.jl | 50 +++++++++++++++---- .../extras/CompilerDevTools/test/runtests.jl | 28 +++++++++++ 2 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 Compiler/extras/CompilerDevTools/test/runtests.jl diff --git a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl index 5d0df5ccaa4e4..b2c31ba9a39e7 100644 --- a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl +++ b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl @@ -2,19 +2,27 @@ module CompilerDevTools using Compiler using Core.IR +using Base: isexpr + +struct SplitCacheOwner + version::Int # use a non-zero value when you don't want to reuse the default cache, e.g. for testing purposes. +end + +SplitCacheOwner() = SplitCacheOwner(0) -struct SplitCacheOwner; end struct SplitCacheInterp <: Compiler.AbstractInterpreter world::UInt + owner::SplitCacheOwner inf_params::Compiler.InferenceParams opt_params::Compiler.OptimizationParams inf_cache::Vector{Compiler.InferenceResult} function SplitCacheInterp(; world::UInt = Base.get_world_counter(), + owner::SplitCacheOwner = SplitCacheOwner(), inf_params::Compiler.InferenceParams = Compiler.InferenceParams(), opt_params::Compiler.OptimizationParams = Compiler.OptimizationParams(), inf_cache::Vector{Compiler.InferenceResult} = Compiler.InferenceResult[]) - new(world, inf_params, opt_params, inf_cache) + new(world, owner, inf_params, opt_params, inf_cache) end end @@ -22,23 +30,43 @@ Compiler.InferenceParams(interp::SplitCacheInterp) = interp.inf_params Compiler.OptimizationParams(interp::SplitCacheInterp) = interp.opt_params Compiler.get_inference_world(interp::SplitCacheInterp) = interp.world Compiler.get_inference_cache(interp::SplitCacheInterp) = interp.inf_cache -Compiler.cache_owner(::SplitCacheInterp) = SplitCacheOwner() +Compiler.cache_owner(interp::SplitCacheInterp) = interp.owner import Core.OptimizedGenerics.CompilerPlugins: typeinf, typeinf_edge -@eval @noinline typeinf(::SplitCacheOwner, mi::MethodInstance, source_mode::UInt8) = - Base.invoke_in_world(which(typeinf, Tuple{SplitCacheOwner, MethodInstance, UInt8}).primary_world, Compiler.typeinf_ext, SplitCacheInterp(; world=Base.tls_world_age()), mi, source_mode) +@eval @noinline typeinf(owner::SplitCacheOwner, mi::MethodInstance, source_mode::UInt8) = + Base.invoke_in_world(which(typeinf, Tuple{SplitCacheOwner, MethodInstance, UInt8}).primary_world, Compiler.typeinf_ext, SplitCacheInterp(; world=Base.tls_world_age(), owner), mi, source_mode) -@eval @noinline function typeinf_edge(::SplitCacheOwner, mi::MethodInstance, parent_frame::Compiler.InferenceState, world::UInt, source_mode::UInt8) +@eval @noinline function typeinf_edge(owner::SplitCacheOwner, mi::MethodInstance, parent_frame::Compiler.InferenceState, world::UInt, source_mode::UInt8) # TODO: This isn't quite right, we're just sketching things for now - interp = SplitCacheInterp(; world) + interp = SplitCacheInterp(; world, owner) Compiler.typeinf_edge(interp, mi.def, mi.specTypes, Core.svec(), parent_frame, false, false) end -function with_new_compiler(f, args...) - mi = @ccall jl_method_lookup(Any[f, args...]::Ptr{Any}, (1+length(args))::Csize_t, Base.tls_world_age()::Csize_t)::Ref{Core.MethodInstance} - world = Base.tls_world_age() +function lookup_method_instance(f, args...) + @ccall jl_method_lookup(Any[f, args...]::Ptr{Any}, (1+length(args))::Csize_t, Base.tls_world_age()::Csize_t)::Ref{Core.MethodInstance} +end + +function Compiler.optimize(interp::SplitCacheInterp, opt::Compiler.OptimizationState, caller::Compiler.InferenceResult) + @invoke Compiler.optimize(interp::Compiler.AbstractInterpreter, opt::Compiler.OptimizationState, caller::Compiler.InferenceResult) + ir = opt.ir::Compiler.IRCode + override = GlobalRef(@__MODULE__(), :with_new_compiler) + for inst in ir.stmts + stmt = inst[:stmt] + isexpr(stmt, :call) || continue + stmt.args[1] === override && continue + insert!(stmt.args, 1, override) + insert!(stmt.args, 3, interp.owner) + end +end + +with_new_compiler(f, args...; owner::SplitCacheOwner = SplitCacheOwner()) = with_new_compiler(f, owner, args...) + +with_new_compiler(f::Core.Builtin, ::SplitCacheOwner, args...) = f(args...) + +function with_new_compiler(f, owner::SplitCacheOwner, args...) + mi = lookup_method_instance(f, args...) new_compiler_ci = Core.OptimizedGenerics.CompilerPlugins.typeinf( - SplitCacheOwner(), mi, Compiler.SOURCE_MODE_ABI + owner, mi, Compiler.SOURCE_MODE_ABI ) invoke(f, new_compiler_ci, args...) end diff --git a/Compiler/extras/CompilerDevTools/test/runtests.jl b/Compiler/extras/CompilerDevTools/test/runtests.jl new file mode 100644 index 0000000000000..9a0bce930b513 --- /dev/null +++ b/Compiler/extras/CompilerDevTools/test/runtests.jl @@ -0,0 +1,28 @@ +using Test +using Compiler: Compiler, code_cache +using Base: get_world_counter, inferencebarrier +using CompilerDevTools +using CompilerDevTools: lookup_method_instance, SplitCacheOwner, SplitCacheInterp + +@activate Compiler + +!@isdefined(cache_version) && (cache_version = 1) +new_cache_version() = SplitCacheOwner(global cache_version += 1) + +do_work(x, y) = x + y +f1() = do_work(inferencebarrier(1), inferencebarrier(2)) + +@testset "CompilerDevTools" begin + owner = new_cache_version() + world = get_world_counter() + interp = SplitCacheInterp(; world, owner) + cache = code_cache(interp) + mi = lookup_method_instance(f1) + @test !haskey(cache, mi) + @test with_new_compiler(f1; owner) === 3 + @test haskey(cache, mi) + # Here `do_work` is compiled at runtime, and so must have + # required extra work to be cached under the same cache owner. + mi = lookup_method_instance(do_work, 1, 2) + @test haskey(cache, mi) +end; From 41fe4b48bd1340ab3ed7f94ec0647bb0af46df0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Wed, 29 Jan 2025 06:06:30 -0500 Subject: [PATCH 2/8] Remove activate --- Compiler/extras/CompilerDevTools/test/runtests.jl | 2 -- 1 file changed, 2 deletions(-) diff --git a/Compiler/extras/CompilerDevTools/test/runtests.jl b/Compiler/extras/CompilerDevTools/test/runtests.jl index 9a0bce930b513..30775662ee8dd 100644 --- a/Compiler/extras/CompilerDevTools/test/runtests.jl +++ b/Compiler/extras/CompilerDevTools/test/runtests.jl @@ -4,8 +4,6 @@ using Base: get_world_counter, inferencebarrier using CompilerDevTools using CompilerDevTools: lookup_method_instance, SplitCacheOwner, SplitCacheInterp -@activate Compiler - !@isdefined(cache_version) && (cache_version = 1) new_cache_version() = SplitCacheOwner(global cache_version += 1) From 1e75edd8960be9d62af4a8aaf3cada74f9050e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Wed, 29 Jan 2025 06:19:12 -0500 Subject: [PATCH 3/8] Don't instrument builtins --- .../extras/CompilerDevTools/src/CompilerDevTools.jl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl index b2c31ba9a39e7..cbabfeb55cd2b 100644 --- a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl +++ b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl @@ -1,6 +1,7 @@ module CompilerDevTools using Compiler +using Compiler: argextype, widenconst using Core.IR using Base: isexpr @@ -53,7 +54,12 @@ function Compiler.optimize(interp::SplitCacheInterp, opt::Compiler.OptimizationS for inst in ir.stmts stmt = inst[:stmt] isexpr(stmt, :call) || continue - stmt.args[1] === override && continue + f = stmt.args[1] + f === override && continue + if isa(f, GlobalRef) + T = widenconst(argextype(f, ir)) + T <: Core.Builtin && continue + end insert!(stmt.args, 1, override) insert!(stmt.args, 3, interp.owner) end @@ -61,8 +67,6 @@ end with_new_compiler(f, args...; owner::SplitCacheOwner = SplitCacheOwner()) = with_new_compiler(f, owner, args...) -with_new_compiler(f::Core.Builtin, ::SplitCacheOwner, args...) = f(args...) - function with_new_compiler(f, owner::SplitCacheOwner, args...) mi = lookup_method_instance(f, args...) new_compiler_ci = Core.OptimizedGenerics.CompilerPlugins.typeinf( From 0a692dcb644d3eae0b5200d64a30e82d1a65cfba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Wed, 5 Feb 2025 09:39:19 -0500 Subject: [PATCH 4/8] Add CompilerDevTools to test suite --- test/choosetests.jl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/choosetests.jl b/test/choosetests.jl index ed441131f061f..ac7ea50722d2d 100644 --- a/test/choosetests.jl +++ b/test/choosetests.jl @@ -6,7 +6,7 @@ const STDLIB_DIR = Sys.STDLIB const STDLIBS = filter!(x -> isfile(joinpath(STDLIB_DIR, x, "src", "$(x).jl")), readdir(STDLIB_DIR)) const TESTNAMES = [ - "subarray", "core", "compiler", "worlds", "atomics", + "subarray", "core", "compiler", "compiler_extras", "worlds", "atomics", "keywordargs", "numbers", "subtype", "char", "strings", "triplequote", "unicode", "intrinsics", "dict", "hashing", "iobuffer", "staged", "offsetarray", @@ -54,6 +54,9 @@ function test_path(test) else return joinpath(pkgdir, "test", "runtests") end + elseif t[1] == "Compiler" && length(t) ≥ 3 && t[2] == "extras" + testpath = length(t) >= 4 ? t[4:end] : ("runtests",) + return joinpath(@__DIR__, "..", t[1], t[2], t[3], "test", testpath...) elseif t[1] == "Compiler" testpath = length(t) >= 2 ? t[2:end] : ("runtests",) return joinpath(@__DIR__, "..", t[1], "test", testpath...) @@ -172,6 +175,7 @@ function choosetests(choices = []) # do subarray before sparse but after linalg filtertests!(tests, "subarray") filtertests!(tests, "compiler", ["Compiler"]) + filtertests!(tests, "compiler_extras", ["Compiler/extras/CompilerDevTools"]) filtertests!(tests, "stdlib", STDLIBS) filtertests!(tests, "internet_required", INTERNET_REQUIRED_LIST) # do ambiguous first to avoid failing if ambiguities are introduced by other tests From 1da9bcf7ccc2364b40b4b9aa7cba6586b52b4934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Tue, 18 Feb 2025 06:48:05 -0500 Subject: [PATCH 5/8] Make SplitCacheOwner a mutable singleton struct --- .../CompilerDevTools/src/CompilerDevTools.jl | 6 +----- .../extras/CompilerDevTools/test/runtests.jl | 20 +++++++------------ 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl index 2062ab9307baf..e52e24d987891 100644 --- a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl +++ b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl @@ -5,11 +5,7 @@ using Compiler: argextype, widenconst using Core.IR using Base: isexpr -struct SplitCacheOwner - version::Int # use a non-zero value when you don't want to reuse the default cache, e.g. for testing purposes. -end - -SplitCacheOwner() = SplitCacheOwner(0) +mutable struct SplitCacheOwner end struct SplitCacheInterp <: Compiler.AbstractInterpreter world::UInt diff --git a/Compiler/extras/CompilerDevTools/test/runtests.jl b/Compiler/extras/CompilerDevTools/test/runtests.jl index 30775662ee8dd..e076de388f96b 100644 --- a/Compiler/extras/CompilerDevTools/test/runtests.jl +++ b/Compiler/extras/CompilerDevTools/test/runtests.jl @@ -1,23 +1,17 @@ using Test -using Compiler: Compiler, code_cache -using Base: get_world_counter, inferencebarrier +using Compiler: code_cache +using Base: inferencebarrier using CompilerDevTools -using CompilerDevTools: lookup_method_instance, SplitCacheOwner, SplitCacheInterp - -!@isdefined(cache_version) && (cache_version = 1) -new_cache_version() = SplitCacheOwner(global cache_version += 1) - -do_work(x, y) = x + y -f1() = do_work(inferencebarrier(1), inferencebarrier(2)) +using CompilerDevTools: lookup_method_instance, SplitCacheInterp @testset "CompilerDevTools" begin - owner = new_cache_version() - world = get_world_counter() - interp = SplitCacheInterp(; world, owner) + do_work(x, y) = x + y + f1() = do_work(inferencebarrier(1), inferencebarrier(2)) + interp = SplitCacheInterp() cache = code_cache(interp) mi = lookup_method_instance(f1) @test !haskey(cache, mi) - @test with_new_compiler(f1; owner) === 3 + @test with_new_compiler(f1, interp.owner) === 3 @test haskey(cache, mi) # Here `do_work` is compiled at runtime, and so must have # required extra work to be cached under the same cache owner. From f04151468ada08f07e8ee6d4092e76203561877f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Tue, 18 Feb 2025 08:23:26 -0500 Subject: [PATCH 6/8] Add test dependencies to project --- Compiler/extras/CompilerDevTools/Project.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Compiler/extras/CompilerDevTools/Project.toml b/Compiler/extras/CompilerDevTools/Project.toml index a2749a9a56a84..39a12bfe518a7 100644 --- a/Compiler/extras/CompilerDevTools/Project.toml +++ b/Compiler/extras/CompilerDevTools/Project.toml @@ -3,3 +3,9 @@ uuid = "92b2d91f-d2bd-4c05-9214-4609ac33433f" [deps] Compiler = "807dbc54-b67e-4c79-8afb-eafe4df6f2e1" + +[extras] +Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" + +[targets] +test = ["Test", "Compiler"] From 6cb06532f76a3c11e09882306b3a7762832f1261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Tue, 18 Feb 2025 10:26:38 -0500 Subject: [PATCH 7/8] Run CompilerDevTools tests with the right environment --- Compiler/extras/CompilerDevTools/test/testpkg.jl | 6 ++++++ test/choosetests.jl | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 Compiler/extras/CompilerDevTools/test/testpkg.jl diff --git a/Compiler/extras/CompilerDevTools/test/testpkg.jl b/Compiler/extras/CompilerDevTools/test/testpkg.jl new file mode 100644 index 0000000000000..b31459219c8dd --- /dev/null +++ b/Compiler/extras/CompilerDevTools/test/testpkg.jl @@ -0,0 +1,6 @@ +using Pkg + +Pkg.activate(dirname(@__DIR__)) do + Pkg.instantiate() + include("runtests.jl") +end diff --git a/test/choosetests.jl b/test/choosetests.jl index ac7ea50722d2d..b07f5e8310ee6 100644 --- a/test/choosetests.jl +++ b/test/choosetests.jl @@ -175,7 +175,7 @@ function choosetests(choices = []) # do subarray before sparse but after linalg filtertests!(tests, "subarray") filtertests!(tests, "compiler", ["Compiler"]) - filtertests!(tests, "compiler_extras", ["Compiler/extras/CompilerDevTools"]) + filtertests!(tests, "compiler_extras", ["Compiler/extras/CompilerDevTools/testpkg"]) filtertests!(tests, "stdlib", STDLIBS) filtertests!(tests, "internet_required", INTERNET_REQUIRED_LIST) # do ambiguous first to avoid failing if ambiguities are introduced by other tests From 24b9359eb2278fdbe62ceecd068a6c4c90f86410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Belmant?= Date: Tue, 18 Feb 2025 12:09:21 -0500 Subject: [PATCH 8/8] Remove unneeded test dependencies --- Compiler/extras/CompilerDevTools/Project.toml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Compiler/extras/CompilerDevTools/Project.toml b/Compiler/extras/CompilerDevTools/Project.toml index 39a12bfe518a7..a2749a9a56a84 100644 --- a/Compiler/extras/CompilerDevTools/Project.toml +++ b/Compiler/extras/CompilerDevTools/Project.toml @@ -3,9 +3,3 @@ uuid = "92b2d91f-d2bd-4c05-9214-4609ac33433f" [deps] Compiler = "807dbc54-b67e-4c79-8afb-eafe4df6f2e1" - -[extras] -Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" - -[targets] -test = ["Test", "Compiler"]