Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support internal cache #545

Merged
merged 5 commits into from
Mar 15, 2024
Merged

Support internal cache #545

merged 5 commits into from
Mar 15, 2024

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Feb 9, 2024

replaces #534
adapts to JuliaLang/julia#52233

@maleadt
Copy link
Member

maleadt commented Feb 27, 2024

Failure on 1.11:

julia> job, _ = Native.create_job(identity, (Int,))
(CompilerJob{NativeCompilerTarget, Main.Native.CompilerParams}(MethodInstance for identity(::Any), CompilerConfig for NativeCompilerTarget, 0x00000000000066fd), Base.Pairs{Symbol, Union{}, Tuple{}, @NamedTuple{}}())

julia> GPUCompiler.code_typed(job)
1-element Vector{Any}:
 CodeInfo(
1 ─     return x
) => Any

julia> Base.code_typed(identity, (Int,))
1-element Vector{Any}:
 CodeInfo(
1 ─     return x
) => Int64

@maleadt
Copy link
Member

maleadt commented Feb 27, 2024

Could we also get some pretty printing back for the new cache? This is pretty sad:

julia> GPUCompiler.ci_cache(job)
Core.Compiler.WorldView{Core.Compiler.InternalCodeCache}(Core.Compiler.InternalCodeCache(GPUCompiler.GPUCompilerCacheToken(NativeCompilerTarget, false, # 0 methods for callable object)), Core.Compiler.WorldRange(0x00000000000066fd, 0x00000000000066fd))

@vchuravy
Copy link
Member Author

vchuravy commented Mar 7, 2024

Could we also get some pretty printing back for the new cache? This is pretty sad:

Not really it is hard (maybe even impossible from Julia) to iterate over all cache entries. Since the entries are now a linked-list in CodeInstance->cache. In the Julia runtime we use a series of nested visitors over the global data-structures to find entries.

So we would need something like jl_foreach_reachable_mtable
foreach_mtable_in_module then followed by a jl_typemap_visitor then visit all the method-instances and then look through all the code instances to print this. Feasible, but annoying.

@vtjnash any other ideas?

@vchuravy
Copy link
Member Author

vchuravy commented Mar 7, 2024

Failure on 1.11:

The issue is less sinister than I feared, but I am not sure jet what went wrong,

julia> job, _ = create_job(identity, (Int,))
(CompilerJob{NativeCompilerTarget, CompilerParams}(MethodInstance for identity(::Any), CompilerConfig for NativeCompilerTarget, 0x000000000000670b), Base.Pairs{Symbol, Union{}, Tuple{}, @NamedTuple{}}())

julia> job.source.specTypes
Tuple{typeof(identity), Any}

So we ended up with the wrong mi.

Edit:

julia> Base.method_instance(identity, (Int,))
MethodInstance for identity(::Any)

Oof.

Edit 2:

julia> @ccall jl_method_lookup(Any[identity, 1]::Ptr{Any}, 2::Csize_t, Base.get_world_counter()::Csize_t)::Ref{Core.MethodInstance}
MethodInstance for identity(::Any)

Edit 3:
Actually the answer seems correct, looking at only(methods(identity)).specializations but our use of specTypes is iffy

Edit 4:
Previously we used Core.Compiler.specialize_method(which(identity, (Int,)), Tuple{Core.Typeof(identity), Int}, Core.svec()) to force a specialization to be created.

@vtjnash
Copy link
Contributor

vtjnash commented Mar 7, 2024

Yeah, I am not sure that pretty-printing a cache object is a sensible thing to try to enumerate

@maleadt maleadt force-pushed the vc/tagged_ci_v2 branch 2 times, most recently from 6d338f6 to b827ece Compare March 13, 2024 12:39
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 82.36%. Comparing base (7229e2b) to head (59d98b0).

❗ Current head 59d98b0 differs from pull request most recent head cbb6de8. Consider uploading reports for the commit cbb6de8 to get more accurate results

Files Patch % Lines
src/jlgen.jl 89.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #545      +/-   ##
==========================================
- Coverage   82.45%   82.36%   -0.09%     
==========================================
  Files          24       24              
  Lines        3322     3335      +13     
==========================================
+ Hits         2739     2747       +8     
- Misses        583      588       +5     

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

@maleadt
Copy link
Member

maleadt commented Mar 13, 2024

With #555 and JuliaLang/julia#53723, this passes tests on 1.11, so we're getting close :-)

@maleadt maleadt merged commit 43dd067 into master Mar 15, 2024
18 of 21 checks passed
@maleadt maleadt deleted the vc/tagged_ci_v2 branch March 15, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants