[Openmp-commits] [PATCH] D127367: [OpenMP] [OMPT] [7/8] Invoke tool-supplied callbacks before and after target launch and data transfer operations

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/src/device.cpp:42
+  void *TgtPtr;                    // Target data ptr
+  ompt_target_data_op_t TgtDataOp; // Data transfer type
+  void beginOp() {
----------------
Style, also names of the arguments.


================
Comment at: openmp/libomptarget/src/interface.cpp:85
+  }
+};
+
----------------
These RAIIs should stay together. If you make them templated you could even get rid of the individual members and potentially have a single RAII to rule them all. (you'd pass the callback rather than the enum, or have a separate switch for the enum to callback translation, either way, you'd then unpack the tuple for the call, see std::apply)


================
Comment at: openmp/libomptarget/src/omptarget.cpp:39
+    if (!OmptEnabled)
+      return;
+    ompt_interface.beginTargetSubmit(NumTeams);
----------------
if you move the conditional into the con/destructor and you create a common base class for these, you should be able to eliminate all but the logic dealing with the provided values (here NumTeams).


================
Comment at: openmp/libomptarget/src/omptarget.cpp:1770
+    int32_t NumTeams = KernelArgs.NumTeams[0] + KernelArgs.NumTeams[1] +
+                       KernelArgs.NumTeams[2];
+    OmptInterfaceTargetSubmitRAII TargetSubmitRAII(NumTeams);
----------------
*, not +


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127367