[Openmp-commits] [PATCH] D95300: [OpenMP][deviceRTLs] Separate declaration of target dependent functions from `target_impl.h`

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

Couple of minor points about redundant function annotations, otherwise good. Thanks for moving the amdgpu code out of the header.

I wonder if the kmpc_impl functions should be consistently extern C, at least when no overloaded.



================
Comment at: openmp/libomptarget/deviceRTLs/common/omptarget.h:333
 
+INLINE DEVICE uint32_t __kmpc_impl_ffs(uint32_t x) { return __builtin_ffs(x); }
+
----------------
tianshilei1992 wrote:
> JonChesterfield wrote:
> > Just INLINE?
> otherwise?
I mean, INLINE expands to a definition that includes DEVICE, so we don't need both


================
Comment at: openmp/libomptarget/deviceRTLs/target_interface.h:27
+// Atomics
+extern DEVICE uint32_t __kmpc_atomic_add(uint32_t *, uint32_t);
+extern DEVICE uint32_t __kmpc_atomic_inc(uint32_t *, uint32_t);
----------------
tianshilei1992 wrote:
> JonChesterfield wrote:
> > I guess these should be EXTERN, not extern DEVICE
> They're C++ functions, so cannot use `EXTERN` directly which is expanded to `extern "C" DEVICE`.
Ah, so 'extern' here is dead. Functions are non-static by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95300