-
Notifications
You must be signed in to change notification settings - Fork 17
Name CUDA kernels based on stack trace in auto_launch
#2376
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
base: main
Are you sure you want to change the base?
Changes from 13 commits
78043ba
687365e
26e4ad5
3a94f7c
f3542d4
cf830bf
e1285ae
06c44e2
7411a8d
9dd3116
93fcf68
25f23ee
9f0fa87
31ec263
eafbe54
52ff458
46c1f6c
fca1ad1
05a5d5b
e7bca7d
a541746
7af8d16
cec2ef7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,8 +2,11 @@ import CUDA | |||||
| import ClimaCore.Fields | ||||||
| import ClimaCore.DataLayouts | ||||||
| import ClimaCore.DataLayouts: empty_kernel_stats | ||||||
| import ClimaCore.DebugOnly: name_kernels_from_stack_trace | ||||||
| import CUDA.GPUCompiler: methodinstance | ||||||
|
|
||||||
| const reported_stats = Dict() | ||||||
| const kernel_names = IdDict() | ||||||
| # Call via ClimaCore.DataLayouts.empty_kernel_stats() | ||||||
| empty_kernel_stats(::ClimaComms.CUDADevice) = empty!(reported_stats) | ||||||
| collect_kernel_stats() = false | ||||||
|
|
@@ -39,19 +42,85 @@ function auto_launch!( | |||||
| always_inline = true, | ||||||
| caller = :unknown, | ||||||
| ) where {F!} | ||||||
| # If desired, compute a kernel name from the stack trace and store in | ||||||
| # a global Dict, which serves as an in memory cache | ||||||
| kernel_name = nothing | ||||||
| if name_kernels_from_stack_trace() | ||||||
| # Create a key from the method instance and types of the args | ||||||
| key = objectid(methodinstance(typeof(f!), typeof(args))) | ||||||
|
||||||
| key = objectid(methodinstance(typeof(f!), typeof(args))) | |
| key = objectid(methodinstance(F!, typeof(args))) |
The function signature could also be changed to
function auto_launch!(
f!::F!,
args::ARGS,
nitems::Union{Integer, Nothing} = nothing;
auto = false,
threads_s = nothing,
blocks_s = nothing,
always_inline = true,
caller = :unknown,
) where {F!, ARGS}
.
.
.
key = objectid(methodinstance(F!, ARGS))
but I think that forced specialization of the method (I'm not sure if that is desirable or not here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These syntax changes may be less clear, so feel free to ignore
kernel_name_exists = haskey(kernel_names, key)
if !kernel_name_exists
or
kernel_name = get!(kernel_names, key) do
#calculate_name_here
end
petebachant marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
imreddyTeja marked this conversation as resolved.
Show resolved
Hide resolved
imreddyTeja marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,4 +171,6 @@ function allow_mismatched_spaces_unsafe() | |
| return false | ||
| end | ||
|
|
||
| name_kernels_from_stack_trace() = false | ||
|
|
||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if importing a package through another package is recommended or not, and I can't find anything when I look online.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does feel wrong. Would it be smarter to make GPUCompiler an explicit dependency of ClimaCore? Is there a nice way to make it optional for just the CUDA extension?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading a little more about this, I guess it would go in
weakdeps?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ClimaLand is a good example of using extensions/weak reps. I'm not sure if there is a way to manage extensions with the package manager though (I never found a way for Julia 1.10), so you might need to edit the Project.toml by hand
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to add the weak dep to ClimaCore with
add --weakin Julia v1.11, but I don't seem to be able to add it to the extension.