[Openmp-commits] [PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeVirtualGPU.cpp:54
+  CGOpenMPRuntime::createOffloadEntry(ID, Addr, Size, Flags, Linkage);
+}
----------------
We should be able to get rid of this file (and the cuda/hip) version. Might be the right time now as a precommit.


================
Comment at: llvm/include/llvm/ADT/Triple.h:166
+    VGPU,
+    LastVendorType = VGPU
   };
----------------
Let's call it OpenMP_VGPU or something like that to make it clear.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2177
+}
+
 /// Abstract Attribute for tracking ICV values.
----------------
@tianshilei1992 This needs a test.


================
Comment at: openmp/libomptarget/DeviceRTL/src/Kernel.cpp:107
+  synchronize::threads();
+
   // Signal the workers to exit the state machine and exit the kernel.
----------------
I don't think we should do this. Instead, the plugin should signal as threads finish the kernel.


================
Comment at: openmp/libomptarget/DeviceRTL/src/Mapping.cpp:171
+#pragma omp begin declare variant match(                                       \
+    device = {arch(x86, x86_64)}, implementation = {extension(match_any)})
+
----------------
We probably should use kind(CPU) or something instead. Nothing x86 specific about it I think.


================
Comment at: openmp/libomptarget/include/DeviceEnvironment.h:83
+
+ThreadEnvironmentTy *getThreadEnvironment(void);
+
----------------
This should go into a new file (ThreadEnvironment)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359