[Openmp-commits] [PATCH] D23587: [OPENMP] Implementation of omp_get_default_device and omp_set_default_device

[Andrey Churbanov via Openmp-commits]

AndreyChurbanov requested changes to this revision.
AndreyChurbanov added a comment.
This revision now requires changes to proceed.

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.


================
Comment at: runtime/src/kmp.h:2253
@@ -2247,2 +2252,3 @@
     kmp_depnode_t *         td_depnode;           // Pointer to graph node if this task has dependencies
+    kmp_int32               td_default_device;    // Default device for task
 #endif
----------------
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.

================
Comment at: runtime/src/kmp_ftn_entry.h:907
@@ -906,3 +906,3 @@
 
-#if KMP_MIC || KMP_OS_DARWIN || defined(KMP_STUB)
+#if KMP_MIC || KMP_OS_DARWIN
 
----------------
Why KMP_STUB is removed here?  Empty routines should continue to work fine for the stubs library I think.

================
Comment at: runtime/src/kmp_tasking.c:864
@@ -863,2 +863,3 @@
     task->td_depnode = NULL;
+    task->td_default_device = __kmp_default_device;
 #endif
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D23587