[Openmp-commits] [PATCH] D84456: [OpenMP] Refactor memory allocation code for easier support for third party memory libraries

[Andrey Churbanov via Phabricator via Openmp-commits]

AndreyChurbanov accepted this revision.
AndreyChurbanov added a comment.
This revision is now accepted and ready to land.

Looks good, besides a few formatting nits.



================
Comment at: openmp/runtime/src/kmp.h:925
+  omp_ata_default = 1,
+  omp_ata__large_cap = 2,
+  omp_ata_const = 3,
----------------
Two underscores in large_cap member looks like a typo.


================
Comment at: openmp/runtime/src/kmp_alloc.cpp:1397
   KE_TRACE(25, ("__kmpc_alloc: T#%d (%d, %p)\n", gtid, (int)size, allocator));
-  al = RCAST(kmp_allocator_t *, CCAST(omp_allocator_handle_t, allocator));
+  if ((uintptr_t) allocator < sizeof(kmp_standard_allocators) / sizeof(kmp_standard_allocators[0]))
+    al = &kmp_standard_allocators[(uintptr_t) allocator];
----------------
Too long line, needs formatting.


================
Comment at: openmp/runtime/src/kmp_alloc.cpp:1416
+  if (al->pool_size > 0) {
+    kmp_uint64 used = KMP_TEST_THEN_ADD64((kmp_int64 *)&al->pool_used, desc.size_a);
       if (used + desc.size_a > al->pool_size) {
----------------
Too long line, please use clang-format to format the change.


================
Comment at: openmp/runtime/src/kmp_alloc.cpp:1420
         KMP_TEST_THEN_ADD64((kmp_int64 *)&al->pool_used, -desc.size_a);
-        if (al->fb == omp_atv_default_mem_fb) {
-          al = (kmp_allocator_t *)omp_default_mem_alloc;
-          ptr = kmp_mk_alloc(*mk_default, desc.size_a);
-        } else if (al->fb == omp_atv_abort_fb) {
-          KMP_ASSERT(0); // abort fallback requested
-        } else if (al->fb == omp_atv_allocator_fb) {
-          KMP_ASSERT(al != al->fb_data);
-          al = al->fb_data;
-          return __kmpc_alloc(gtid, size, (omp_allocator_handle_t)al);
-        } // else ptr == NULL;
-      } else {
-        // pool has enough space
-        ptr = kmp_mk_alloc(*al->memkind, desc.size_a);
-        if (ptr == NULL) {
-          if (al->fb == omp_atv_default_mem_fb) {
-            al = (kmp_allocator_t *)omp_default_mem_alloc;
-            ptr = kmp_mk_alloc(*mk_default, desc.size_a);
-          } else if (al->fb == omp_atv_abort_fb) {
-            KMP_ASSERT(0); // abort fallback requested
-          } else if (al->fb == omp_atv_allocator_fb) {
-            KMP_ASSERT(al != al->fb_data);
-            al = al->fb_data;
-            return __kmpc_alloc(gtid, size, (omp_allocator_handle_t)al);
-          }
-        }
+	switch (al->fb) {
+	default:
----------------
Please replace tabs with 8 spaces in this code block.


================
Comment at: openmp/runtime/src/kmp_settings.cpp:3312
     num = __kmp_str_to_int(buf, *next);
-    KMP_ASSERT(num > 0);
-    switch (num) {
-    case 4:
-      if (__kmp_memkind_available) {
-        __kmp_def_allocator = omp_high_bw_mem_alloc;
-      } else {
-        __kmp_msg(kmp_ms_warning,
-                  KMP_MSG(OmpNoAllocator, "omp_high_bw_mem_alloc"),
-                  __kmp_msg_null);
-        __kmp_def_allocator = omp_default_mem_alloc;
+    KMP_ASSERT(num > 0 && num < (int) (sizeof(__kmp_allocator_names)/sizeof(__kmp_allocator_names[0]) + 1));
+  } else {
----------------
Too long line, please format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84456/new/

https://reviews.llvm.org/D84456