[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Support/Triple.cpp:512
+      .Case("oe", Triple::OpenEmbedded)
+      .Case("vgpu", Triple::OpenMP_VGPU)
+      .Default(Triple::UnknownVendor);
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Debug.cpp:53
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+int32_t vprintf(const char *, void *);
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Kernel.cpp:128
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+void __kmpc_target_deinit(IdentTy *Ident, int8_t Mode, bool) {
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Mapping.cpp:28
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Synchronization.cpp:290
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Synchronization.cpp:314
+// Simply call fenceKernel because there is no need to sync with host
+void fenceSystem(int) { fenceKernel(0); }
+
----------------
Pass the memory order, also rename the arguments to match the coding convention.


================
Comment at: openmp/libomptarget/DeviceRTL/src/Synchronization.cpp:317
+void syncWarp(__kmpc_impl_lanemask_t Mask) {
+  getThreadEnvironment()->syncWarp();
+}
----------------
Pass the mask


================
Comment at: openmp/libomptarget/DeviceRTL/src/Utils.cpp:56
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+
----------------



================
Comment at: openmp/libomptarget/DeviceRTL/src/Utils.cpp:68
+
+#pragma omp end declare variant
+
----------------
Can't we merge this with AMDGPU?


================
Comment at: openmp/libomptarget/DeviceRTL/src/Utils.cpp:138
+#pragma omp begin declare variant match(                                       \
+    device = {kind(cpu)}, implementation = {extension(match_any)})
+
----------------



================
Comment at: openmp/libomptarget/plugins/vgpu/src/rtl.cpp:303
+                    TeamIdx += NumCTAs;
+                  }
+
----------------
Can we split this up and create some helper functions maybe?


================
Comment at: openmp/libomptarget/src/rtl.cpp:34
+    /* Virtual GPU target   */ "libomptarget.rtl.vgpu.so",
 };
 
----------------
Introduce an environment variable, if it is set, X86 target should skip the image.
Also, add a TODO such that we later look into the image and inspect it to decide automatically.


================
Comment at: openmp/libomptarget/test/lit.cfg:189
         config.substitutions.append(("%libomptarget-compile-and-run-" + \
             libomptarget_target, \
             "echo ignored-command"))
----------------
Leftovers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359