Skip to content

Conversation

adityas-amd
Copy link

OpenMP: Fix refcounting for implicit pointer-hop entries in libomptarget (ROCm#287)

This change prevents implicit pointer-only hops (compiler-generated chained pointer members) from incrementing/decrementing reference counts in libomptarget’s mapping table. Without this, intermediate implicit entries can linger after exit data, blocking proper cleanup.

Chained mappings like s.ps->ps->ps->ps->i generate implicit member pointer hops.
These hops are entries with no explicit TO/FROM/ALWAYS/DELETE and size == 0. they get refcounted as regular objects, leaving intermediate entries with non-zero refs after exit.

Problem:
After target enter data, implicit hops +1 refcount.
On target exit data, decrements don’t fully clear the intermediate hops.
Result: stale intermediate mappings prevent proper deletion and leak entries.

Solution
Detect implicit pointer-hop entries and do not update refcounts for them in begin/end paths.
Symmetric handling in targetDataBegin and targetDataEnd to avoid negative counts and ensure full cleanup.

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2025

@llvm/pr-subscribers-offload

Author: Aditya Srichandan (adityas-amd)

Changes

OpenMP: Fix refcounting for implicit pointer-hop entries in libomptarget (ROCm#287)

This change prevents implicit pointer-only hops (compiler-generated chained pointer members) from incrementing/decrementing reference counts in libomptarget’s mapping table. Without this, intermediate implicit entries can linger after exit data, blocking proper cleanup.

Chained mappings like s.ps->ps->ps->ps->i generate implicit member pointer hops.
These hops are entries with no explicit TO/FROM/ALWAYS/DELETE and size == 0. they get refcounted as regular objects, leaving intermediate entries with non-zero refs after exit.

Problem:
After target enter data, implicit hops +1 refcount.
On target exit data, decrements don’t fully clear the intermediate hops.
Result: stale intermediate mappings prevent proper deletion and leak entries.

Solution
Detect implicit pointer-hop entries and do not update refcounts for them in begin/end paths.
Symmetric handling in targetDataBegin and targetDataEnd to avoid negative counts and ensure full cleanup.


Full diff: https://github.com/llvm/llvm-project/pull/163878.diff

1 Files Affected:

  • (modified) offload/libomptarget/omptarget.cpp (+12)
diff --git a/offload/libomptarget/omptarget.cpp b/offload/libomptarget/omptarget.cpp
index 39286d41ec865..08a83c193b196 100644
--- a/offload/libomptarget/omptarget.cpp
+++ b/offload/libomptarget/omptarget.cpp
@@ -41,6 +41,15 @@ using llvm::SmallVector;
 #ifdef OMPT_SUPPORT
 using namespace llvm::omp::target::ompt;
 #endif
+static inline bool isImplicitPointerHop(int64_t MapType, int64_t Size) {
+  //Pointer hop entries flagged as PTR and MEMBER_OF
+  const bool IsPointer  = (MapType & OMP_TGT_MAPTYPE_PTR) != 0;
+  const bool IsMemberOf = (MapType & OMP_TGT_MAPTYPE_MEMBER_OF) != 0;
+  const bool HasExplicit =
+      (MapType & (OMP_TGT_MAPTYPE_TO | OMP_TGT_MAPTYPE_FROM |
+                  OMP_TGT_MAPTYPE_ALWAYS | OMP_TGT_MAPTYPE_DELETE)) != 0;
+  return IsPointer && IsMemberOf && !HasExplicit && Size == 0;
+}
 
 int AsyncInfoTy::synchronize() {
   int Result = OFFLOAD_SUCCESS;
@@ -567,6 +576,9 @@ int targetDataBegin(ident_t *Loc, DeviceTy &Device, int32_t ArgNum,
     // may be considered a hack, we could revise the scheme in the future.
     bool UpdateRef =
         !(ArgTypes[I] & OMP_TGT_MAPTYPE_MEMBER_OF) && !(FromMapper && I == 0);
+    if (IsImplicit && isImplicitPointerHop(ArgTypes[I], ArgSizes[I])) {
+         UpdateRef = false;
+    }
 
     MappingInfoTy::HDTTMapAccessorTy HDTTMap =
         Device.getMappingInfo().HostDataToTargetMap.getExclusiveAccessor();

@abhinavgaba
Copy link
Contributor

The code-generation for pointer/pointee mapping is being updated, and will no longer involve mapping intermediate pointers. Mapping intermediate pointers is not OpenMP compliant.

Please see #153683, and try your test with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants