Allow for generic MethodTableView #494

vchuravy · 2023-08-16T23:03:31Z

Currently, SPIRVIntrinsics, OpenCL, oneAPI, and POCL all share one method-table. We would like to disambiguate package specific overlays from the actual common intrinsic definitions.

codecov · 2023-08-16T23:12:09Z

Codecov Report

Attention: Patch coverage is 90.69767% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.71%. Comparing base (636d916) to head (bf8e2b8).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/jlgen.jl	90.47%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   73.54%   73.71%   +0.17%     
==========================================
  Files          24       24              
  Lines        3485     3519      +34     
==========================================
+ Hits         2563     2594      +31     
- Misses        922      925       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

vchuravy · 2025-06-11T08:42:58Z

For now just rebased and included the code from Shenanigans.jl

github-actions · 2025-06-11T08:43:02Z

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.

diff --git a/src/interface.jl b/src/interface.jl
index 7553dbb..cb0c05f 100644
--- a/src/interface.jl
+++ b/src/interface.jl
@@ -231,12 +231,14 @@ isintrinsic(@nospecialize(job::CompilerJob), fn::String) = false
 # provide a specific interpreter to use.
 if VERSION >= v"1.11.0-DEV.1552"
 get_interpreter(@nospecialize(job::CompilerJob)) =
-    GPUInterpreter(job.world; method_table_view=maybe_cached(method_table_view(job)),
+        GPUInterpreter(
+        job.world; method_table_view = maybe_cached(method_table_view(job)),
                    token=ci_cache_token(job), inf_params=inference_params(job),
                    opt_params=optimization_params(job))
 else
 get_interpreter(@nospecialize(job::CompilerJob)) =
-    GPUInterpreter(job.world; method_table_view=maybe_cached(method_table_view(job)),
+        GPUInterpreter(
+        job.world; method_table_view = maybe_cached(method_table_view(job)),
                    code_cache=ci_cache(job), inf_params=inference_params(job),
                    opt_params=optimization_params(job))
 end
diff --git a/src/jlgen.jl b/src/jlgen.jl
index 1bff8a0..4982c64 100644
--- a/src/jlgen.jl
+++ b/src/jlgen.jl
@@ -300,7 +300,7 @@ Base.Experimental.@MethodTable(GLOBAL_METHOD_TABLE)
 # Implements a priority lookup for method tables, where the first match in the stack get's returned.
 # An alternative to this would be to use a "Union" where we would query the parent method table and
 # do a most-specific match.
-struct StackedMethodTable{MTV<:CC.MethodTableView} <: CC.MethodTableView
+struct StackedMethodTable{MTV <: CC.MethodTableView} <: CC.MethodTableView
     world::UInt
     mt::Core.MethodTable
     parent::MTV
@@ -313,7 +313,7 @@ CC.isoverlayed(::StackedMethodTable) = true
 @static if VERSION >= v"1.11.0-DEV.363"
     # https://github.com/JuliaLang/julia/pull/51078
     # same API as before but without returning isoverlayed flag
-    function CC.findall(@nospecialize(sig::Type), table::StackedMethodTable; limit::Int=-1)
+    function CC.findall(@nospecialize(sig::Type), table::StackedMethodTable; limit::Int = -1)
         result = CC._findall(sig, table.mt, table.world, limit)
         result === nothing && return nothing # to many matches
         nr = CC.length(result)
@@ -321,17 +321,19 @@ CC.isoverlayed(::StackedMethodTable) = true
             # no need to fall back to the parent method view
             return result
         end
-    
+
         parent_result = CC.findall(sig, table.parent; limit)::Union{Nothing, CC.MethodLookupResult}
         parent_result === nothing && return nothing #too many matches
-    
+
         # merge the parent match results with the internal method table
         return CC.MethodLookupResult(
             CC.vcat(result.matches, parent_result.matches),
             CC.WorldRange(
                 CC.max(result.valid_worlds.min_world, parent_result.valid_worlds.min_world),
-                CC.min(result.valid_worlds.max_world, parent_result.valid_worlds.max_world)),
-            result.ambig | parent_result.ambig)
+                CC.min(result.valid_worlds.max_world, parent_result.valid_worlds.max_world)
+            ),
+            result.ambig | parent_result.ambig
+        )
     end
 
     function CC.findsup(@nospecialize(sig::Type), table::StackedMethodTable)
@@ -342,11 +344,12 @@ CC.isoverlayed(::StackedMethodTable) = true
             parent_match,
             CC.WorldRange(
                 max(valid_worlds.min_world, parent_valid_worlds.min_world),
-                min(valid_worlds.max_world, parent_valid_worlds.max_world))
-            )
+                min(valid_worlds.max_world, parent_valid_worlds.max_world)
+            ),
+        )
     end
 else
-    function CC.findall(@nospecialize(sig::Type), table::StackedMethodTable; limit::Int=-1)
+    function CC.findall(@nospecialize(sig::Type), table::StackedMethodTable; limit::Int = -1)
         result = CC._findall(sig, table.mt, table.world, limit)
         result === nothing && return nothing # to many matches
         nr = CC.length(result)
@@ -354,22 +357,25 @@ else
             # no need to fall back to the parent method view
             return CC.MethodMatchResult(result, true)
         end
-    
+
         parent_result = CC.findall(sig, table.parent; limit)::Union{Nothing, CC.MethodMatchResult}
         parent_result === nothing && return nothing #too many matches
-    
+
         overlayed = parent_result.overlayed | !CC.isempty(result)
         parent_result = parent_result.matches::CC.MethodLookupResult
-        
+
         # merge the parent match results with the internal method table
         return CC.MethodMatchResult(
-        CC.MethodLookupResult(
-            CC.vcat(result.matches, parent_result.matches),
-            CC.WorldRange(
-                CC.max(result.valid_worlds.min_world, parent_result.valid_worlds.min_world),
-                CC.min(result.valid_worlds.max_world, parent_result.valid_worlds.max_world)),
-            result.ambig | parent_result.ambig),
-        overlayed)
+            CC.MethodLookupResult(
+                CC.vcat(result.matches, parent_result.matches),
+                CC.WorldRange(
+                    CC.max(result.valid_worlds.min_world, parent_result.valid_worlds.min_world),
+                    CC.min(result.valid_worlds.max_world, parent_result.valid_worlds.max_world)
+                ),
+                result.ambig | parent_result.ambig
+            ),
+            overlayed
+        )
     end
 
     function CC.findsup(@nospecialize(sig::Type), table::StackedMethodTable)
@@ -380,8 +386,10 @@ else
             parent_match,
             CC.WorldRange(
                 max(valid_worlds.min_world, parent_valid_worlds.min_world),
-                min(valid_worlds.max_world, parent_valid_worlds.max_world)),
-            overlayed)
+                min(valid_worlds.max_world, parent_valid_worlds.max_world)
+            ),
+            overlayed,
+        )
     end
 end
 
@@ -404,7 +412,7 @@ end
 
 get_method_table_view(world::UInt, mt::CC.MethodTable) = CC.OverlayMethodTable(world, mt)
 
-struct GPUInterpreter{MTV<:CC.MethodTableView} <: CC.AbstractInterpreter
+struct GPUInterpreter{MTV <: CC.MethodTableView} <: CC.AbstractInterpreter
     world::UInt
     method_table_view::MTV
 
@@ -421,7 +429,7 @@ end
 
 @static if HAS_INTEGRATED_CACHE
 function GPUInterpreter(world::UInt=Base.get_world_counter();
-                        method_table_view::CC.MethodTableView,
+            method_table_view::CC.MethodTableView,
                         token::Any,
                         inf_params::CC.InferenceParams,
                         opt_params::CC.OptimizationParams)
@@ -429,19 +437,21 @@ function GPUInterpreter(world::UInt=Base.get_world_counter();
 
     inf_cache = Vector{CC.InferenceResult}()
 
-    return GPUInterpreter(world, method_table_view,
+        return GPUInterpreter(
+            world, method_table_view,
                           token, inf_cache,
                           inf_params, opt_params)
 end
 
 function GPUInterpreter(interp::GPUInterpreter;
                         world::UInt=interp.world,
-                        method_table_view::Core.MethodTable=interp.method_table_view,
+            method_table_view::Core.MethodTable = interp.method_table_view,
                         token::Any=interp.token,
                         inf_cache::Vector{CC.InferenceResult}=interp.inf_cache,
                         inf_params::CC.InferenceParams=interp.inf_params,
                         opt_params::CC.OptimizationParams=interp.opt_params)
-    return GPUInterpreter(world, method_table_view,
+        return GPUInterpreter(
+            world, method_table_view,
                           token, inf_cache,
                           inf_params, opt_params)
 end
@@ -449,7 +459,7 @@ end
 else
 
 function GPUInterpreter(world::UInt=Base.get_world_counter();
-                        method_table_view::CC.MethodTableView,
+            method_table_view::CC.MethodTableView,
                         code_cache::CodeCache,
                         inf_params::CC.InferenceParams,
                         opt_params::CC.OptimizationParams)
@@ -457,19 +467,21 @@ function GPUInterpreter(world::UInt=Base.get_world_counter();
 
     inf_cache = Vector{CC.InferenceResult}()
 
-    return GPUInterpreter(world, method_table_view,
+        return GPUInterpreter(
+            world, method_table_view,
                           code_cache, inf_cache,
                           inf_params, opt_params)
 end
 
 function GPUInterpreter(interp::GPUInterpreter;
                         world::UInt=interp.world,
-                        method_table_view::CC.MethodTableView=interp.method_table_view,
+            method_table_view::CC.MethodTableView = interp.method_table_view,
                         code_cache::CodeCache=interp.code_cache,
                         inf_cache::Vector{CC.InferenceResult}=interp.inf_cache,
                         inf_params::CC.InferenceParams=interp.inf_params,
                         opt_params::CC.OptimizationParams=interp.opt_params)
-    return GPUInterpreter(world, method_table_view,
+        return GPUInterpreter(
+            world, method_table_view,
                           code_cache, inf_cache,
                           inf_params, opt_params)
 end
diff --git a/test/utils.jl b/test/utils.jl
index f0de138..f70cd94 100644
--- a/test/utils.jl
+++ b/test/utils.jl
@@ -111,14 +111,14 @@ DoubleStackedMT() = StackedMethodTable(Base.get_world_counter(), OtherMT, LayerM
         @test isoverlayed(DoubleStackedMT()) == true
     end
 
-    o_sin  = findsup(Tuple{typeof(sin), Float64}, OverlayMT())
-    s_sin  = findsup(Tuple{typeof(sin), Float64}, StackedMT())
+    o_sin = findsup(Tuple{typeof(sin), Float64}, OverlayMT())
+    s_sin = findsup(Tuple{typeof(sin), Float64}, StackedMT())
     ss_sin = findsup(Tuple{typeof(sin), Float64}, DoubleStackedMT())
     @test s_sin == o_sin
     @test ss_sin == o_sin
 
-    o_sin  = findall(Tuple{typeof(sin), Float64}, OverlayMT())
-    s_sin  = findall(Tuple{typeof(sin), Float64}, StackedMT())
+    o_sin = findall(Tuple{typeof(sin), Float64}, OverlayMT())
+    s_sin = findall(Tuple{typeof(sin), Float64}, StackedMT())
     ss_sin = findall(Tuple{typeof(sin), Float64}, DoubleStackedMT())
     if VERSION >= v"1.11.0-DEV.363"
         @test o_sin.matches == s_sin.matches
@@ -150,8 +150,8 @@ next_world = Base.get_world_counter()
     @test worlds.min_world > prev_world
     @test worlds.max_world == typemax(typeof(next_world))
 
-    o_sin  = findall(Tuple{typeof(sin), Float64}, OverlayMT())
-    s_sin  = findall(Tuple{typeof(sin), Float64}, StackedMT())
+    o_sin = findall(Tuple{typeof(sin), Float64}, OverlayMT())
+    s_sin = findall(Tuple{typeof(sin), Float64}, StackedMT())
     ss_sin = findall(Tuple{typeof(sin), Float64}, DoubleStackedMT())
     if VERSION >= v"1.11.0-DEV.363"
         @test o_sin.matches == s_sin.matches

vchuravy · 2025-06-16T15:50:31Z

@aviatesk can you take a look if this makes sense? The goal here is to have a stack of three method tables, the first layer superseding the second, the second superseding the third. The third is Julia's internal method table.

test/utils.jl

aviatesk

From an implementation perspective, this seems fine.
One point to note is that in the current overlay method table mehcanism, simply looking at .fully_covers cannot reproduce the specialization behavior that occurs in normal dispatch.
Consider the following example:

Base.Experimental.@MethodTable mt

func(a::Number) = :number

func(a::Integer) = :integer

Base.Experimental.@overlay mt func(a::Number) = :number_overlay

In this case, the correct specialization for Tuple{typeof(func), Int} should naturally be dispatch to func(::Integer):

julia> Base.Compiler.findall(Tuple{typeof(func),Int}, Base.Compiler.InternalMethodTable(Base.get_world_counter()))
Compiler.MethodLookupResult(Any[Core.MethodMatch(Tuple{typeof(func), Int64}, svec(), func(a::Integer) @ Main REPL[14]:1, true)], Compiler.WorldRange(0x00000000000097cd, 0xffffffffffffffff), false)

However, when using overlay method tables, this rule is not preserved, and something like "local specialization within the overlay method table" takes in place:

julia> result = Base.Compiler.findall(Tuple{typeof(func),Int}, Base.Compiler.OverlayMethodTable(Base.get_world_counter(), mt))
Compiler.MethodLookupResult(Any[Core.MethodMatch(Tuple{typeof(func), Int64}, svec(), func(a::Number) @ Main REPL[15]:1, true)], Compiler.WorldRange(0x00000000000097ce, 0xffffffffffffffff), false)

julia> result.matches[1].fully_covers
true

This issue likely applies to this PR as well. It might be worth being careful about whether there are any problems regarding this point. This is more of an implementation issue with Base.Compiler.findall(type, ::OverlayMethodTable) rather than a problem on the side that uses OverlayMethodTable though.

vchuravy · 2025-06-20T16:56:09Z

This issue likely applies to this PR as well. It might be worth being careful about whether there are any problems regarding this point.

For our uses that is fine, but yes for some of the potential use-cases that might be an issue.

Base automatically changed from vc/mt_cleanup to master August 17, 2023 18:10

maleadt force-pushed the master branch from 628b8dd to f3f2c5e Compare September 5, 2023 19:11

maleadt force-pushed the master branch 5 times, most recently from 1d233d7 to e18b7c2 Compare January 20, 2025 10:33

vchuravy force-pushed the vc/mtv branch from 931e788 to 02f1270 Compare June 11, 2025 08:33

vchuravy marked this pull request as ready for review June 11, 2025 08:42

vchuravy force-pushed the vc/mtv branch from bfc2be3 to 3b101e0 Compare June 16, 2025 15:36

vchuravy mentioned this pull request Jun 16, 2025

Use stacked method tables JuliaGPU/OpenCL.jl#325

Open

vchuravy mentioned this pull request Jun 17, 2025

Use stacked method tables JuliaGPU/oneAPI.jl#506

Draft

vchuravy commented Jun 17, 2025

View reviewed changes

test/utils.jl Show resolved Hide resolved

vchuravy mentioned this pull request Jun 17, 2025

Use stacked method tables JuliaGPU/KernelAbstractions.jl#615

Open

aviatesk reviewed Jun 18, 2025

View reviewed changes

vchuravy added 5 commits June 23, 2025 14:09

Allow for generic MethodTableView

750e249

Add StackedMethodTable

3a98406

move overlay action to top-level

93a53ba

check larger than prev_world instead of next_world

0cc29a7

add deprecation note

bf8e2b8

maleadt force-pushed the vc/mtv branch from 51888d0 to bf8e2b8 Compare June 23, 2025 12:09

vchuravy merged commit 79e0f56 into master Jun 24, 2025
19 of 22 checks passed

vchuravy deleted the vc/mtv branch June 24, 2025 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow for generic MethodTableView #494

Allow for generic MethodTableView #494

vchuravy commented Aug 16, 2023 •

edited

Loading

Uh oh!

codecov bot commented Aug 16, 2023 •

edited

Loading

Uh oh!

vchuravy commented Jun 11, 2025

Uh oh!

github-actions bot commented Jun 11, 2025 •

edited

Loading

Uh oh!

vchuravy commented Jun 16, 2025

Uh oh!

Uh oh!

aviatesk left a comment

Uh oh!

vchuravy commented Jun 20, 2025

Uh oh!

Uh oh!

Uh oh!

Allow for generic MethodTableView #494

Allow for generic MethodTableView #494

Conversation

vchuravy commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vchuravy commented Jun 11, 2025

Uh oh!

github-actions bot commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vchuravy commented Jun 16, 2025

Uh oh!

Uh oh!

aviatesk left a comment

Choose a reason for hiding this comment

Uh oh!

vchuravy commented Jun 20, 2025

Uh oh!

Uh oh!

Uh oh!

vchuravy commented Aug 16, 2023 •

edited

Loading

codecov bot commented Aug 16, 2023 •

edited

Loading

github-actions bot commented Jun 11, 2025 •

edited

Loading