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

[George Rokos via Phabricator via Openmp-commits]

grokos added inline comments.


================
Comment at: openmp/libomptarget/plugins/ve/src/rtl.cpp:183
+
+#ifdef __cplusplus
+extern "C" {
----------------
JonChesterfield wrote:
> 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.
I second that. All `__tgt_rtl_*` functions are declared as `extern "C"` in `omptargetplugin.h`, so their definitions do not need to do the same.


================
Comment at: openmp/libomptarget/plugins/ve/src/rtl.cpp:232-236
+  // Is the library version incompatible with the header file?
+  if (elf_version(EV_CURRENT) == EV_NONE) {
+    DP("Incompatible ELF library!\n");
+    return NULL;
+  }
----------------
libomptarget first checks whether we have a valid binary for this plugin (`__tgt_rtl_is_valid_binary`) and if yes then it will try to load it (`__tgt_rtl_load_binary`). So this EFL check is guaranteed to have been executed as part of the `__tgt_rtl_is_valid_binary` call, it can be removed from here.


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

https://reviews.llvm.org/D76843