JonChesterfield added a comment.
I can't see it in the diff - does the cmake somewhere enable the existing tests
on this new target?
A bit surprised to see ffi involved, are we thinking of spawning a separate
process for the target?
================
Comment at: clang/lib/Basic/Targets/X86.h:49
+static const unsigned X86VGPUAddrSpaceMap[] = {
+ 0, // Default
----------------
It's not clear to me what this is x86 specific. Being able to run our tests on
power / arm etc seems like an advantage. Would also mean we would avoid adding
openmp stuff the x86 specific files. Maybe OpenMPVGPUAddrSpaceMap and put it in
one of the openmp source files?
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3988
+ (T.isNVPTX() || T.isAMDGCN() ||
+ T.getVendor() == llvm::Triple::OpenMP_VGPU) &&
Args.hasArg(options::OPT_fopenmp_cuda_mode);
----------------
Add a isOpenmpVGPU function?
================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:135
-I${devicertl_base_directory}/../include
+ -I${devicertl_base_directory}/../plugins/vgpu/src
${LIBOMPTARGET_LLVM_INCLUDE_DIRS_DEVICERTL}
----------------
Should only add this include to the vgu, not all the plugins. May be able to
use relative include paths to drop it entirely
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/new/
https://reviews.llvm.org/D113359
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
