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

[libomptarget] document ActionFunctions in the amdgpu plugin. #66397

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Sep 14, 2023

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 14, 2023

@llvm/pr-subscribers-backend-amdgpu

Changes None -- Full diff: https://github.com//pull/66397.diff

1 Files Affected:

  • (modified) openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp (+9-2)
diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
index 7b4ea794fe7f0d2..6e16621c71c1afe 100644
--- a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
@@ -1055,7 +1055,7 @@ struct AMDGPUStreamTy {
     return false;
   }
 
-  // Callback for host-to-host memory copies.
+  // Callback for host-to-host memory copies. This is an asynchronous action.
   static Error memcpyAction(void *Data) {
     MemcpyArgsTy *Args = reinterpret_cast<MemcpyArgsTy *>(Data);
     assert(Args && "Invalid arguments");
@@ -1067,7 +1067,12 @@ struct AMDGPUStreamTy {
     return Plugin::success();
   }
 
-  // Callback for releasing a memory buffer to a memory manager.
+  /// Releasing a memory buffer to a memory manager. This is a post completion action.
+  /// There are two kinds of memory buffers:
+  ///   1. For kernel arguments. This buffer can be freed after receiving the kernel completion signal.
+  ///   2. For H2D tranfers that need pinned memory space for staging. This buffer can be freed after receiving the transfer completion signal.
+  ///   3. For D2H tranfers that need pinned memory space for staging. This buffer cannot be freed after receiving the transfer completion signal because of the following asynchronous H2H callback.
+  ///      For this reason, This action can only be taken at AMDGPUStreamTy::complete()
   static Error releaseBufferAction(void *Data) {
     ReleaseBufferArgsTy *Args = reinterpret_cast<ReleaseBufferArgsTy *>(Data);
     assert(Args && "Invalid arguments");
@@ -1078,6 +1083,8 @@ struct AMDGPUStreamTy {
     return Args->MemoryManager->deallocate(Args->Buffer);
   }
 
+  /// Releasing a signal object back to SignalManage. This is a post completion action.
+  /// This action can only be taken at AMDGPUStreamTy::complete()
   static Error releaseSignalAction(void *Data) {
     ReleaseSignalArgsTy *Args = reinterpret_cast<ReleaseSignalArgsTy *>(Data);
     assert(Args && "Invalid arguments");

@ye-luo ye-luo merged commit 8c2da6b into llvm:main Sep 14, 2023
2 checks passed
@ye-luo ye-luo deleted the more-comments branch September 14, 2023 17:19
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants