Implementation of missing OpenMP 4.0 API functions omp_get_default_device and omp_set_default_device.
Also, added support for the environment variable OMP_DEFAULT_DEVICE.
Details
- Reviewers
jlpeyton hbae tlwilmar AndreyChurbanov - Commits
- rG28f31b405e0d: [OPENMP] Implementation of omp_get_default_device and omp_set_default_device
rOMP281065: [OPENMP] Implementation of omp_get_default_device and omp_set_default_device
rL281065: [OPENMP] Implementation of omp_get_default_device and omp_set_default_device
Diff Detail
- Repository
- rL LLVM
Event Timeline
I suggest moving the ICV into kmp_internal_control structure, thus its propagation from parent to children is automatically maintained everywhere in the library code.
runtime/src/kmp.h | ||
---|---|---|
2253 ↗ | (On Diff #68277) | This does not look like a good location for the ICV. Is it better to place it along with other per-task ICVs in the td_icvs structure? Thus it will be automatically maintained at each task creation using appropriate mechanism. |
runtime/src/kmp_ftn_entry.h | ||
907 ↗ | (On Diff #68277) | Why KMP_STUB is removed here? Empty routines should continue to work fine for the stubs library I think. |
runtime/src/kmp_tasking.c | ||
864 ↗ | (On Diff #68277) | This looks wrong, as the ICV should be inherited from the parent task (if any). As I written below, the ICV should better be placed and initialized along with other per-task ICVs. Then global value can be assigned in __kmp_get_global_icvs() function. |
In general: Do we really need this in libomp? We currently have the problem of two runtime libraries and will probably continue with it: libomp supporting the host and liboffload / libomptarget supporting all target related things.
I imagine in the current state this will give linker errors with liboffload / libomptarget that define their own version of this function.
Not necessarily, but this looks like an appropriate solution - to move the functions from offload library to libomp.
We currently have the problem of two runtime libraries and will probably continue with it: libomp supporting the host and liboffload / libomptarget supporting all target related things.
I imagine in the current state this will give linker errors with liboffload / libomptarget that define their own version of this function.
It is unlikely because the functions supposed to be removed from offload library simultaneously, at list this is an intention.
An alternative would be to implement internal versions of the functions in libomp leaving public symbols in offload library and changing their implementation to call to libomp, but this also does not look very elegant solution.
The initial problem might be clearer specified in the summary, as it is not an implementation of missing functions, it is the (part of) movement of incorrectly implemented functions to place where they can easier be implemented correctly. The offload library does not mange OMP tasks and thus it can hardly implement per-task control in a correct way without help of libomp.
As discussed last week, libomptarget does not implement these two functions. The 4.0 standard mandates that the default device is set per-task. libomptarget (and liboffload as far as I understand) has no notion of tasks, so implementing those functions anywhere else than libomp would be problematic.
runtime/src/kmp_ftn_entry.h | ||
---|---|---|
907 ↗ | (On Diff #68277) | Right, I had misunderstood the role of KMP_STUB. I restored it and merged both versions of each routine into a single function, following the style of other API functions. |
Hmm, that wasn't really part of the discussion but anyway I now understand why this has to be done here (I was under the impression that there would only be one default-device-var per program).
I assume you will then call omp_get_default_device() from within the libomptarget?