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

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added inline comments.


================
Comment at: runtime/src/kmp_tasking.cpp:1692
+// complete
+kmp_int32 __kmpc_omp_taskwait(ident_t *loc_ref, kmp_int32 gtid) {
+#if OMPT_SUPPORT && OMPT_OPTIONAL
----------------
hbae wrote:
> Hahnfeld wrote:
> > hbae wrote:
> > > I know this is ugly, but we couldn't find a better way to fix ~15% regression in 376.kdtree.
> > Duplicating code increases the risk of inconsistencies. Can we use templates so that the compiler can do the work? Something like the following:
> > 
> > ```lang=C++
> > template <bool ompt>
> > kmp_int32 __kmpc_omp_taskwait_temp(ident_t *loc_ref, kmp_int32 gtid) {
> >   ...
> > }
> > 
> > kmp_int32 __kmpc_omp_taskwait(ident_t *loc_ref, kmp_int32 gtid) {
> > #if OMPT_SUPPORT && OMPT_OPTIONAL
> >   if (UNLIKELY(ompt_enabled.enabled)) {
> >     return __kmpc_omp_taskwait_temp<true>(loc_ref, gtid);
> >   }
> > #endif
> >   return __kmpc_omp_taskwait_temp<false>(loc_ref, gtid);
> > }
> > ```
> Yes, we can try this, but I suggest to do it in a separate patch since it requires performance validation.
But this solution is a maintenance nightmare. If we need time to do performance tests, then the "unoptimized" version should go in at first. So only one function that has all the instrumentation in it


https://reviews.llvm.org/D38185