search

OpenMPToolsInterface / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies. Note: the repository does not accept github pull requests at this moment. Please submit your patches at http://reviews.llvm.org.
http://llvm.org
2 stars 4 forks source link

ICV names as per OpenMP 5.1 and support for tool-verbose-init-var ICV #33

Closed jinisusan closed 3 years ago

jinisusan commented 3 years ago

Changes to incorporate the new ICV names as per TR9 (sans the ompd prefix), some cleanup and changes to support the ompd retrieval of the tool-verbose-init-var ICV.

jprotze commented 3 years ago

Is there a specific reason, why you reference TR9 and not 5.1? I don't recall any change which would impact this patch.

The spec says in 5.5.9.1 ompd_enumerate_icvs:

The ICVs num-procs-var, thread-num-var, final-task-var, implicit-task-var and team-size-var must also be available with an ompd- prefix.

The intention was, that we didn't want to remove the old names without deprecation (and Debuggers @jdelsign might already match for the old names?). Looking at your patch, we missed the change ompd-implicit-var -> implicit-task-var. This name change would not be addressed by above sentence.

To comply with the spec, you should add back the ompd- variants to the FOREACH_OMPD_ICV like:

 macro (ompd_num_procs_var, "ompd-num-procs-var", ompd_scope_address_space, 0)   \

In ompd_get_icv_from_scope / ompd_get_icv_string_from_scope you can just duplicate the cases and reuse the function call:

      case ompd_icv_num_procs_var:
      case ompd_icv_ompd_num_procs_var:
        return ompd_get_num_procs((ompd_address_space_handle_t*)handle, icv_value);

I guess for compatibility with 5.0 we should have ompd-implicit-var and for 5.1 compliance ompd-implicit-task-var.

jinisusan commented 3 years ago

Thanks for pointing this out, @jprotze ! I have made the necessary changes. (includes a change to include ompd-final-var also -- like ompd-implicit-var). There is no specific reason to mention TR9. (I assumed that was the convention -- since I saw other TR references in a couple of places).

jprotze commented 3 years ago

There is no specific reason to mention TR9. (I assumed that was the convention -- since I saw other TR references in a couple of places).

We typically referenced TR documents, when the TR was the latest released document, to make clear that the change relates to something newer than the latest released spec version.

lgtm now, thanks!

jinisusan commented 3 years ago

Thank you !