[Openmp-commits] [PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert marked 5 inline comments as done.
jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:27
+
+/// The target region _kernel_ interface for GPUs
+///
----------------
ABataev wrote:
> All exported functions are declared in the `interface.h` file. I don't think we need an extra interface file here
`interface.h`, or to be more precise for people that do not know, `deviceRTLs/nvptx/src/interface.h`, is nvptx specific. This file, `deviceRTLs/common/target_region.h`, is by design target agnostic and not placed _under_ the nvptx subfolder. If you are willing to move `interface.h` into a common space and remove the nvptx specific functions we can merge the two. Otherwise, I have strong reservations agains that and good reason not to do it.


================
Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:100
+///
+EXTERN int8_t __kmpc_target_region_kernel_init(bool UseSPMDMode,
+                                               bool RequiresOMPRuntime,
----------------
ABataev wrote:
> Better to use `ident_loc` for passing info about execution mode and full/lightweight runtime.
Could you please explain why you think that? Adding indirection through a structure does not really seem beneficial to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59319