[Openmp-commits] [PATCH] D76843: [Openmp] Libomptarget plugin for NEC SX-Aurora

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

Looks pretty good to me. Thin shim over veo_*.



================
Comment at: openmp/libomptarget/plugins/ve/CMakeLists.txt:43
+    "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../exports")
+
+  # Report to the parent scope that we are building a plugin.
----------------
Worth considering adding `"-Wl,-z,defs"` here to detect unresolved symbols up front.


================
Comment at: openmp/libomptarget/plugins/ve/src/rtl.cpp:47
+
+#include "../../common/elf_common.c"
+
----------------
Nice. I hadn't noticed this file.


================
Comment at: openmp/libomptarget/plugins/ve/src/rtl.cpp:183
+
+#ifdef __cplusplus
+extern "C" {
----------------
I think a few plugins do this, just found it in hsa. It's stylistically unusual though. omptargetplugin.h declares the API under extern "C", and there's an export file that internalises the other symbols, so I'm pretty sure this #ifdef block and the one at the end can be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76843