[Openmp-commits] [PATCH] D133447: [Libomptarget] Implement OpenMP 5.2 semantics for device pointers

[Ye Luo via Phabricator via Openmp-commits]

ye-luo requested changes to this revision.
ye-luo added inline comments.
This revision now requires changes to proceed.


================
Comment at: openmp/libomptarget/include/device.h:285
+    /// If the pointer is present in the mapping table.
+    unsigned IsPresent : 1;
+  } Flags = {0, 0, 0};
----------------
IsPresent is not orthogonal to IsHostPointer. This unamed struct design is flawed.

Also directly access TargetPointerResultTy member is also bad. There is lack of read only protection. accessing members should via a function.

something like.
struct TargetPointerResultTy {
  std::unique_ptr<EntryFlags> flags# null is the entry doesn't exist.

  inline bool isPresent() const { return bool(flags); }
}


================
Comment at: openmp/libomptarget/test/mapping/implicit_device_ptr.c:12
+int main(void) {
+  int *A = omp_target_alloc(sizeof(int), omp_get_default_device());
+
----------------
Doesn't compile. Need `(int *) omp_target_alloc`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133447/new/

https://reviews.llvm.org/D133447