From 4515c28488f1cfcba5588d81fcb11deea3784ad5 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 17 Feb 2025 11:51:28 +0100 Subject: [PATCH 1/9] Use support for gc_safe=true in Base if available --- src/ccalls.jl | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index 3fe88e0..7ab71e8 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -91,13 +91,29 @@ render_arg(io, arg::Base.RefValue{T}) where {T} = print(io, "Ref{", T, "}") ## version of ccall that calls jl_gc_safe_enter|leave around the inner ccall - -# TODO: replace with JuliaLang/julia#49933 once merged - # note that this is generally only safe with functions that do not call back into Julia. # when callbacks occur, the code should ensure the GC is not running by wrapping the code # in the `@gcunsafe` macro +const HAS_CCALL_GCSAFE = VERSION >= v"1.13.0-DEV.70" + +if HAS_CCALL_GCSAFE + """ + @gcsafe_ccall ... + + Call a foreign function just like `@ccall`, but marking it safe for the GC to run. This is + useful for functions that may block, so that the GC isn't blocked from running, but may also + be required to prevent deadlocks (see JuliaGPU/CUDA.jl#2261). + + Note that this is generally only safe with non-Julia C functions that do not call back into + Julia. When using callbacks, the code should make sure to transition back into GC-unsafe + mode using the `@gcunsafe` macro. + """ + macro gcsafe_ccall(exprs) + exprs = pushfirst(exprs, :(gc_safe = true)) + return Base.ccall_macro_lower((:ccall), Base.ccall_macro_parse(exprs)...) + end +else function ccall_macro_lower(func, rettype, types, args, nreq) # instead of re-using ccall or Expr(:foreigncall) to perform argument conversion, # we need to do so ourselves in order to insert a jl_gc_safe_enter|leave @@ -152,3 +168,4 @@ mode using the `@gcunsafe` macro. macro gcsafe_ccall(expr) return ccall_macro_lower(Base.ccall_macro_parse(expr)...) end +end # HAS_CCALL_GCSAFE From 2a280de5b1887778891f59f4358215bcaf164a57 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 17 Feb 2025 15:26:05 +0100 Subject: [PATCH 2/9] fixup! Use support for gc_safe=true in Base if available --- src/ccalls.jl | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index 7ab71e8..cf87c1d 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -97,18 +97,20 @@ render_arg(io, arg::Base.RefValue{T}) where {T} = print(io, "Ref{", T, "}") const HAS_CCALL_GCSAFE = VERSION >= v"1.13.0-DEV.70" -if HAS_CCALL_GCSAFE - """ - @gcsafe_ccall ... +""" +@gcsafe_ccall ... - Call a foreign function just like `@ccall`, but marking it safe for the GC to run. This is - useful for functions that may block, so that the GC isn't blocked from running, but may also - be required to prevent deadlocks (see JuliaGPU/CUDA.jl#2261). +Call a foreign function just like `@ccall`, but marking it safe for the GC to run. This is +useful for functions that may block, so that the GC isn't blocked from running, but may also +be required to prevent deadlocks (see JuliaGPU/CUDA.jl#2261). - Note that this is generally only safe with non-Julia C functions that do not call back into - Julia. When using callbacks, the code should make sure to transition back into GC-unsafe - mode using the `@gcunsafe` macro. - """ +Note that this is generally only safe with non-Julia C functions that do not call back into +Julia. When using callbacks, the code should make sure to transition back into GC-unsafe +mode using the `@gcunsafe` macro. +""" +macro gcsafe_ccall end + +if HAS_CCALL_GCSAFE macro gcsafe_ccall(exprs) exprs = pushfirst(exprs, :(gc_safe = true)) return Base.ccall_macro_lower((:ccall), Base.ccall_macro_parse(exprs)...) @@ -154,17 +156,6 @@ function ccall_macro_lower(func, rettype, types, args, nreq) end end -""" - @gcsafe_ccall ... - -Call a foreign function just like `@ccall`, but marking it safe for the GC to run. This is -useful for functions that may block, so that the GC isn't blocked from running, but may also -be required to prevent deadlocks (see JuliaGPU/CUDA.jl#2261). - -Note that this is generally only safe with non-Julia C functions that do not call back into -Julia. When using callbacks, the code should make sure to transition back into GC-unsafe -mode using the `@gcunsafe` macro. -""" macro gcsafe_ccall(expr) return ccall_macro_lower(Base.ccall_macro_parse(expr)...) end From 4c7bee13343642dce7f5fde1626316a772787237 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 11:50:56 +0100 Subject: [PATCH 3/9] Update src/ccalls.jl Co-authored-by: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> --- src/ccalls.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index cf87c1d..d71b3ab 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -95,7 +95,7 @@ render_arg(io, arg::Base.RefValue{T}) where {T} = print(io, "Ref{", T, "}") # when callbacks occur, the code should ensure the GC is not running by wrapping the code # in the `@gcunsafe` macro -const HAS_CCALL_GCSAFE = VERSION >= v"1.13.0-DEV.70" +const HAS_CCALL_GCSAFE = VERSION >= v"1.13.0-DEV.70" || v"1.12-DEV.2029" <= VERSION < v"1.13-" """ @gcsafe_ccall ... From 664146a593f472b1ae1b91ad76ab938638944250 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:21:44 +0100 Subject: [PATCH 4/9] add test --- src/ccalls.jl | 3 +-- test/runtests.jl | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index d71b3ab..d2e4cea 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -105,8 +105,7 @@ useful for functions that may block, so that the GC isn't blocked from running, be required to prevent deadlocks (see JuliaGPU/CUDA.jl#2261). Note that this is generally only safe with non-Julia C functions that do not call back into -Julia. When using callbacks, the code should make sure to transition back into GC-unsafe -mode using the `@gcunsafe` macro. +the Julia directly. """ macro gcsafe_ccall end diff --git a/test/runtests.jl b/test/runtests.jl index 2a93553..77195ed 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -42,5 +42,19 @@ using GPUToolbox @test !(sv2 > sv2) # Default end - # TODO: @debug_ccall and @gcsafe_ccall tests + @testset "gcsafe_ccall" begin + function gc_safe_ccall() + # jl_rand is marked as JL_NOTSAFEPOINT + @gcsafe_ccall gc_safe=true jl_rand()::UInt64 + end + + let llvm = sprint(code_llvm, gc_safe_ccall, ()) + # check that the call works + @test gc_safe_ccall() isa UInt64 + # check for the gc_safe store + @test occursin("store atomic i8 2", llvm) + end + end + + # TODO: @debug_ccall tests end From b0391876fa73d9df148a6fb59fc3981fca6ead88 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:34:42 +0100 Subject: [PATCH 5/9] fixup! add test --- test/runtests.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 77195ed..00bea1e 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -45,9 +45,9 @@ using GPUToolbox @testset "gcsafe_ccall" begin function gc_safe_ccall() # jl_rand is marked as JL_NOTSAFEPOINT - @gcsafe_ccall gc_safe=true jl_rand()::UInt64 + @gcsafe_ccall jl_rand()::UInt64 end - + let llvm = sprint(code_llvm, gc_safe_ccall, ()) # check that the call works @test gc_safe_ccall() isa UInt64 From a0d2e0bccf84b3113bf469fa57ac3b1af4ab920b Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:36:58 +0100 Subject: [PATCH 6/9] add InteractiveUtils to tests --- test/Project.toml | 1 + test/runtests.jl | 1 + 2 files changed, 2 insertions(+) diff --git a/test/Project.toml b/test/Project.toml index 7fb0db5..b91e6e3 100644 --- a/test/Project.toml +++ b/test/Project.toml @@ -1,4 +1,5 @@ [deps] +InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [extras] diff --git a/test/runtests.jl b/test/runtests.jl index 00bea1e..3b9364d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,5 +1,6 @@ using Test using GPUToolbox +using InteractiveUtils @testset "GPUToolbox.jl" begin @testset "SimpleVersion" begin From 91d7d6f4e7edd6946733770843a71d8ea9279494 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:44:30 +0100 Subject: [PATCH 7/9] fix tests --- src/ccalls.jl | 76 ++++++++++++++++++++++++------------------------ test/runtests.jl | 9 ++++-- 2 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index d2e4cea..f72a003 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -111,51 +111,51 @@ macro gcsafe_ccall end if HAS_CCALL_GCSAFE macro gcsafe_ccall(exprs) - exprs = pushfirst(exprs, :(gc_safe = true)) + exprs = pushfirst!(exprs, :(gc_safe = true)) return Base.ccall_macro_lower((:ccall), Base.ccall_macro_parse(exprs)...) end else -function ccall_macro_lower(func, rettype, types, args, nreq) - # instead of re-using ccall or Expr(:foreigncall) to perform argument conversion, - # we need to do so ourselves in order to insert a jl_gc_safe_enter|leave - # just around the inner ccall - - cconvert_exprs = [] - cconvert_args = [] - for (typ, arg) in zip(types, args) - var = gensym("$(func)_cconvert") - push!(cconvert_args, var) - push!(cconvert_exprs, :($var = Base.cconvert($(esc(typ)), $(esc(arg))))) - end + function ccall_macro_lower(func, rettype, types, args, nreq) + # instead of re-using ccall or Expr(:foreigncall) to perform argument conversion, + # we need to do so ourselves in order to insert a jl_gc_safe_enter|leave + # just around the inner ccall + + cconvert_exprs = [] + cconvert_args = [] + for (typ, arg) in zip(types, args) + var = gensym("$(func)_cconvert") + push!(cconvert_args, var) + push!(cconvert_exprs, :($var = Base.cconvert($(esc(typ)), $(esc(arg))))) + end - unsafe_convert_exprs = [] - unsafe_convert_args = [] - for (typ, arg) in zip(types, cconvert_args) - var = gensym("$(func)_unsafe_convert") - push!(unsafe_convert_args, var) - push!(unsafe_convert_exprs, :($var = Base.unsafe_convert($(esc(typ)), $arg))) - end + unsafe_convert_exprs = [] + unsafe_convert_args = [] + for (typ, arg) in zip(types, cconvert_args) + var = gensym("$(func)_unsafe_convert") + push!(unsafe_convert_args, var) + push!(unsafe_convert_exprs, :($var = Base.unsafe_convert($(esc(typ)), $arg))) + end - call = quote - $(unsafe_convert_exprs...) + call = quote + $(unsafe_convert_exprs...) - gc_state = @ccall(jl_gc_safe_enter()::Int8) - ret = ccall( - $(esc(func)), $(esc(rettype)), $(Expr(:tuple, map(esc, types)...)), - $(unsafe_convert_args...) - ) - @ccall(jl_gc_safe_leave(gc_state::Int8)::Cvoid) - ret - end + gc_state = @ccall(jl_gc_safe_enter()::Int8) + ret = ccall( + $(esc(func)), $(esc(rettype)), $(Expr(:tuple, map(esc, types)...)), + $(unsafe_convert_args...) + ) + @ccall(jl_gc_safe_leave(gc_state::Int8)::Cvoid) + ret + end - return quote - @inline - $(cconvert_exprs...) - GC.@preserve $(cconvert_args...) $(call) + return quote + @inline + $(cconvert_exprs...) + GC.@preserve $(cconvert_args...) $(call) + end end -end -macro gcsafe_ccall(expr) - return ccall_macro_lower(Base.ccall_macro_parse(expr)...) -end + macro gcsafe_ccall(expr) + return ccall_macro_lower(Base.ccall_macro_parse(expr)...) + end end # HAS_CCALL_GCSAFE diff --git a/test/runtests.jl b/test/runtests.jl index 3b9364d..a6a430c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -52,8 +52,13 @@ using InteractiveUtils let llvm = sprint(code_llvm, gc_safe_ccall, ()) # check that the call works @test gc_safe_ccall() isa UInt64 - # check for the gc_safe store - @test occursin("store atomic i8 2", llvm) + if !GPUToolbox.HAS_CCALL_GCSAFE && VERSION >= v"1.11" + # check for the gc_safe store + @test occursin("jl_gc_safe_enter", llvm) + @test occursin("jl_gc_safe_leave", llvm) + else + @test occursin("store atomic i8 2", llvm) + end end end From 0aa70615d0286ae18b5b3529ef16ea0e4e3829c9 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:45:54 +0100 Subject: [PATCH 8/9] fix impl --- src/ccalls.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ccalls.jl b/src/ccalls.jl index f72a003..94d9182 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -110,8 +110,8 @@ the Julia directly. macro gcsafe_ccall end if HAS_CCALL_GCSAFE - macro gcsafe_ccall(exprs) - exprs = pushfirst!(exprs, :(gc_safe = true)) + macro gcsafe_ccall(expr) + exprs = Any[:(gc_safe = true), expr] return Base.ccall_macro_lower((:ccall), Base.ccall_macro_parse(exprs)...) end else From 362bd749baf84a4683aed0dd16d66dff8a0c333e Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Mon, 10 Mar 2025 12:56:03 +0100 Subject: [PATCH 9/9] fix test on 1.10 --- test/runtests.jl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index a6a430c..d26ca7f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -52,12 +52,15 @@ using InteractiveUtils let llvm = sprint(code_llvm, gc_safe_ccall, ()) # check that the call works @test gc_safe_ccall() isa UInt64 - if !GPUToolbox.HAS_CCALL_GCSAFE && VERSION >= v"1.11" - # check for the gc_safe store - @test occursin("jl_gc_safe_enter", llvm) - @test occursin("jl_gc_safe_leave", llvm) - else - @test occursin("store atomic i8 2", llvm) + # v1.10 is hard to test since ccall are just raw runtime pointers + if VERSION >= v"1.11" + if !GPUToolbox.HAS_CCALL_GCSAFE + # check for the gc_safe store + @test occursin("jl_gc_safe_enter", llvm) + @test occursin("jl_gc_safe_leave", llvm) + else + @test occursin("store atomic i8 2", llvm) + end end end end