This is an archive of the discontinued LLVM Phabricator instance.

Bug fixes in OMPT support
ClosedPublic

Authored by jmellorcrummey on Jul 9 2015, 7:38 AM.

Details

Summary
  1. in kmp_csupport.c, move computation of parameters only needed for OMPT tracing inside a conditional to reduce overhead if not receiving ompt_event_master_begin callbacks.
  2. in kmp_gsupport.c
    • remove spurious reset of OMPT reenter_runtime_frame (which is set in its caller, GOMP_parallel_start
    • correct placement of #if OMP_TRACE so that state is maintained even if tracing support not included.
  3. in z_Linux_util.c
    • add architecture independent support for OMPT by setting and resetting OMPT's exit_frame_ptr before and after invoking a microtask.
  4. On the Intel MIC, the loader refuses to retain static symbols in the libomp.so shared library, even though tools need them. The loader could not be bullied into doing so. To accommodate this, I changed the visibility of OMPT placeholder functions to public. This required additions in exports.so.txt, adding extern "C" scoping in ompt-general.c so that the public placeholder symbols won't be mangled.

Diff Detail

Repository
rL LLVM

Event Timeline

jmellorcrummey retitled this revision from to Bug fixes in OMPT support.
jmellorcrummey updated this object.
jmellorcrummey added a reviewer: jcownie.
jmellorcrummey set the repository for this revision to rL LLVM.
jmellorcrummey added a subscriber: llvm-commits.
jcownie edited edge metadata.Jul 9 2015, 7:47 AM

It looks generally fine, but I am nervous about putting the ompt state placeholders in OpenMP's namespace (by giving them thename prefix "omp_" ). Is there a reason not to name them "ompt_", which would move them into our namespace?

jmellorcrummey edited edge metadata.
jmellorcrummey removed rL LLVM as the repository for this revision.

Moved OMPT placeholders from omp namespace to ompt namespace.

jmellorcrummey set the repository for this revision to rL LLVM.Jul 9 2015, 9:10 AM
jlpeyton accepted this revision.Jul 9 2015, 12:15 PM
jlpeyton added a reviewer: jlpeyton.
jlpeyton added a subscriber: jlpeyton.

LGTM as well. In the future, can you provide more context to the patch. Details can be seen here: llvm.org/docs/Phabricator.html

This revision is now accepted and ready to land.Jul 9 2015, 12:15 PM
This revision was automatically updated to reflect the committed changes.