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

Dhruva Chakrabarti via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Apr 27 09:34:13 PDT 2023

dhruvachak added inline comments.

Comment at: openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp:67
+  /// Connect plugin instance with libomptarget
+  static OmptLibraryConnectorTy LibomptargetConnector("libomptarget");
+  static ompt_start_tool_result_t OmptResult;
I think LibomptargetConnector and OmptResult need not be statics, locals will suffice. The connector object is used to copy the callback function pointers from libomptarget and once that is done, the object is not required any more. The original intent of the static was to guard against accidental repeat connect attempts. If we add the assert to the finalizer like I suggested, that will still be guarded against. Regarding OmptResult, it is used to communicate some pointers to the other library which caches the pointers, so no need to keep the wrapper object around.

We could make similar changes to the connect logic in libomptarget and add an assert during finalizer registration in libomp but let's keep that for a separate commit, if at all.

Comment at: openmp/libomptarget/src/OmptCallback.cpp:39
+  LibomptargetRtlFinalizer() : FiniFn(nullptr) {}
+  void registerRtl(ompt_finalize_t fn) { FiniFn = fn; }
+  void finalize() {
In registerRtl, before assigning to FiniFn, add an assert that it is nullptr.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list