Skip to content

Commit

Permalink
Avoid host allocations for by-offload arguments to GPU kernels (chape…
Browse files Browse the repository at this point in the history
…l-lang#24970)

This PR addresses a performance issue first noted by @stonea in the
context of Coral. Today, I had a chat with @Guillaume-Helbecque who was
also suffering partly from it.

This issue arises when:
1. multiple GPUs in a node are used, and
2. LICM is not able to peel off the array metadata before we generate
the GPU kernel

When (2), we pass arrays by offload -- we copy the record onto GPU
memory and pass that record as an argument to the kernel. This involves
allocating memory for the record on device-accessible memory. Now, most
likely because oversight (on my part), that allocation is done on
page-locked host memory instead. (Probably because I was thinking that
object instances should be host-accessible so that they can be
initialized, but we're talking about an already initialized record that
we'll bit-copy anyways).

Page-locked allocations are globally synchronous. In other words, they
will block all other GPUs, or be blocked until all other GPUs are free.
That is why this is especially problematic with (1).

This PR adjusts the memory allocation to be on device memory proper.

Resolves chapel-lang#24936

[Reviewed by @stonea]

Test:
- [x] nvidia
- [x] amd
- [x] some EX testing as I noted it to be problematic in the past
  • Loading branch information
e-kayrakli authored Aug 29, 2024
2 parents 5a7a77f + 8e4afeb commit 9d058ee
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions runtime/src/chpl-gpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,19 @@ static void cfg_add_offload_param(kernel_cfg* cfg, void* arg, size_t size) {

cfg->param_dyn_allocated[i] = true;

// the kernel_params array must store the addresses of things that will be
// copied to the device memory before the kernel launch. For by-offload
// params, we need it to store the address of the pointer generated by
// chpl_gpu_mem_array_alloc. We need to allocate dynamic memory for that
// address-to-pointer as well, because otherwise the address (which is just
// another void*) will go out of scope at the end of this function.
cfg->kernel_params[i] = chpl_mem_alloc(sizeof(void*),
CHPL_RT_MD_GPU_KERNEL_PARAM, cfg->ln,
cfg->fn);

// TODO this doesn't work on EX, why?
// *kernel_params[i] = chpl_gpu_impl_mem_array_alloc(cur_arg_size, stream);
*(cfg->kernel_params[i]) = chpl_gpu_mem_alloc(size, CHPL_RT_MD_GPU_KERNEL_ARG,
cfg->ln, cfg->fn);
*(cfg->kernel_params[i]) = chpl_gpu_mem_array_alloc(size,
CHPL_RT_MD_GPU_KERNEL_ARG,
cfg->ln, cfg->fn);

chpl_gpu_impl_copy_host_to_device(*(cfg->kernel_params[i]), arg, size,
cfg->stream);
Expand Down

0 comments on commit 9d058ee

Please sign in to comment.