[Openmp-commits] [PATCH] D38185: Implementation of OMPT as specified in OpenMP 5.0 Preview 1

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim marked 59 inline comments as done.
protze.joachim added a comment.

Marked comments that are solved by latest diff as done.



================
Comment at: runtime/src/include/50/ompt.h.var:386
+    ompt_target_data_alloc = 1,
+    ompt_target_data_transfer_to_dev = 2,
+    ompt_target_data_transfer_from_dev = 3,
----------------
omalyshe wrote:
> I suggest using *_device instead of *_dev
This name is given by the spec. If you really perfer this modification, please create an issue in the OMPT spec repository


================
Comment at: runtime/src/include/50/ompt.h.var:512
+    ompt_cancel_sections       = 0x2,
+    ompt_cancel_do             = 0x4,
+    ompt_cancel_taskgroup      = 0x8,
----------------
omalyshe wrote:
> Probably, ompt_cancel_loop is better
I agree, this would be better. But we need to change the spec first, before we update this in the implementation.


================
Comment at: runtime/test/lit.cfg:126-128
+config.substitutions.append(("%filecheck-threads", "FileCheck --check-prefixes " + "THREADS"))
+config.substitutions.append(("%filecheck-custom", "FileCheck "))
+config.substitutions.append(("%filecheck", "FileCheck --check-prefixes " + config.filecheck_prefixes))
----------------
Hahnfeld wrote:
> No, please keep the prefixes in the tests so that it is clearly visible which prefixes are checked!
The idea of CHECK-WAIT is to allow testing of a runtime, that does not support "optional" events.
If OMPT_OPTIONAL is off, only CHECK, but non of the optional CHECK-X should be checked


https://reviews.llvm.org/D38185