[Openmp-commits] [PATCH] D124070: [OpenMP] [OMPT] [amdgpu] [4/8] Implemented callback registration in amdgpu plugin

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp:22
+#include "ompt_device_callbacks.h"
+
+/// Object maintaining all the callbacks in the plugin
----------------
Can we put this stuff into a namespace (ompt::) rather than prefixing it with Ompt?


================
Comment at: openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp:71
+  static OmptLibraryConnectorTy LibomptargetConnector("libomptarget");
+  static ompt_start_tool_result_t OmptResult;
+
----------------
Multiple plugins can be active, are you sure you want static variables here?


================
Comment at: openmp/libomptarget/src/rtl.cpp:75
+    InitOmptLibomp();
+  #endif
+
----------------
Is this patch run through clang-format (only the patch part)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124070