[PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface
Johannes Doerfert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 13 14:37:15 PDT 2019
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
> 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,
> 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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits