Skip to content

Expose function to check if precompile guaranteed to fail (similar to hasmethod) #228

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

Draft
wants to merge 1 commit into
base: v1.10.2+RAI
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2048,7 +2048,6 @@ function _require_from_serialized(uuidkey::PkgId, path::String, ocachepath::Unio
end



# relative-path load

"""
Expand Down Expand Up @@ -3208,6 +3207,27 @@ macro __DIR__()
return isempty(_dirname) ? pwd() : abspath(_dirname)
end

# TODO: mark `public`?
# TODO: name? isprecompilable? has_precompilable_specialization? has_precompilable_method?
# TODO: put here next to `precompile` or in `reflection.jl` next to `hasmethod`?
"""
precompilable(f, argtypes::Tuple{Vararg{Any}})::Bool

Check if we can find a method instance for the given function `f` for the argument tuple
(of types) `argtypes` that we can try to compile, but do not compile it.

If `false`, then `precompile(f, argtypes)` would return `false`.
If `true`, then `precompile(f, argtypes)` would try to compile a method specialization (but
may still return `false` if it does not succeed).
"""
function precompilable(@nospecialize(f), @nospecialize(argtypes::Tuple))
precompile(Tuple{Core.Typeof(f), argtypes...})
end
Comment on lines +3223 to +3225
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could call this has_method_instance_match?


function precompilable(@nospecialize(argt::Type))
return ccall(:jl_has_compile_hint_specialization, Int32, (Any,), argt) != 0
end

"""
precompile(f, argtypes::Tuple{Vararg{Any}})

Expand Down
13 changes: 13 additions & 0 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2942,6 +2942,19 @@ JL_DLLEXPORT int jl_compile_hint(jl_tupletype_t *types)
return 1;
}

// true if jl_get_compile_hint_specialization can find a method instance for us to try
// to compile
JL_DLLEXPORT int jl_has_compile_hint_specialization(jl_tupletype_t *types)
Copy link
Member Author

@nickrobinson251 nickrobinson251 Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just jl_compile_hint from above (which is what precompile(f, argtypes) calls), but without the jl_compile_method_instance call

{
size_t world = jl_atomic_load_acquire(&jl_world_counter);
size_t min_valid = 0;
size_t max_valid = ~(size_t)0;
jl_method_instance_t *mi = jl_get_compile_hint_specialization(types, world, &min_valid, &max_valid, 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jl_get_compile_hint_specialization first finds a "compilable method match" then calls jl_method_match_to_mi to create a "method instance" -- if that step can't return NULL then i suppose we don't need to spend the time creating the "method instance" and could just check if we found a "method match" ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah interesting. i'm not sure whether that step is generally nullable or not. 🤔

The code you have here, now, roughly matches what i had in mind when you described this idea to me 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kpamnany is going to ask Gabriel about this

if (mi == NULL)
return 0;
return 1;
}

// add type of `f` to front of argument tuple type
jl_value_t *jl_argtype_with_function(jl_value_t *f, jl_value_t *types0)
{
Expand Down
14 changes: 14 additions & 0 deletions test/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1057,3 +1057,17 @@ end
@test !Base.ismutationfree(Vector{UInt64})

@test Base.ismutationfree(Type{Union{}})

module PrecompilableTest
foo(v::AbstractVector) = first(v) +1
end
@testset "precompilable" begin
@test !Base.precompilable(PrecompilableTest.foo, (AbstractVector,))
@test !Base.precompilable(PrecompilableTest.foo, (AbstractVector{Int},))
@test !Base.precompilable(PrecompilableTest.foo, (Vector,))
@test Base.precompilable(PrecompilableTest.foo, (Vector{Int},))
@test hasmethod(PrecompilableTest.foo, (AbstractVector,))
@test hasmethod(PrecompilableTest.foo, (AbstractVector{Int},))
@test hasmethod(PrecompilableTest.foo, (Vector,))
@test hasmethod(PrecompilableTest.foo, (Vector{Int},))
end
Comment on lines +1061 to +1073
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to add tests for all the false positive and false negative cases in the PR description?