Page MenuHomePhabricator

[OpenMP] Implement 5.0 memory management
ClosedPublic

Authored by jlpeyton on Mar 25 2019, 10:49 AM.

Details

Summary
  • Replace HBWMALLOC API with more general MEMKIND API, new functions and variables added.
  • Have libmemkind.so loaded when accessible.
  • Redirect memspaces to default one except for high bandwidth which is processed separately.
  • Ignore some allocator traits e.g., sync_hint, access, pinned, while others are processed normally e.g., alignment, pool_size, fallback, fb_data, partition.
  • Add tests for memory management

Patch by Andrey Churbanov

Diff Detail

Repository
rOMP OpenMP

Event Timeline

jlpeyton created this revision.Mar 25 2019, 10:49 AM
ABataev added inline comments.
runtime/src/include/50/omp.h.var
293

By default, the base type for enum is int. Value UINTPTR_MAX is too big for int.

AndreyChurbanov added inline comments.
runtime/src/include/50/omp.h.var
293

Alexey, theoretically you are correct. But the majority of compilers (gcc, clang, icc, maybe others) support pointer-size enum by default. The OpenMP language committee decided that this type must be enumeration in order to allow strict type checking by compilers. And at the same time it is supposed to keep pointer value as a convenient way to designate custom memory allocator or custom memory space object.

An alternative would require mapping of pointers to integers inside runtime library that looks a worse solution comparing to utilizing common non-standard language extension.

Compilers those do not support pointer-size enumeration can use implementation under #if above, that is currently specified for cl and icl (and maybe others) on Windows. This breaks OpenMP specification requirement, but again looks preferable comparing to mapping pointers to integers inside the runtime.

If you still think this is a big problem, then we can think of changing the OpenMP specification to not require enumeration types here, and allow integers instead. But this is also not a simple and time consuming procedure.

ABataev added inline comments.Mar 26 2019, 4:06 AM
runtime/src/include/50/omp.h.var
293

I think it is better to use based enum explicitly, if the compiler supports it. I think you need to add a check for c++11 and use enum omp_allocator_handle_t : uintptr_t explicitly.

jlpeyton updated this revision to Diff 192665.Mar 28 2019, 9:54 AM
  1. Addressed Alexey’s comment on enumeration type in C++11 - added base type under “#if __cplusplus >= 201103”.
  2. Fixed Fortan headers so that they can be built by gfortran – removed bind(c) from omp_init_allocator function declaration (gfortran complained that array of structures is not c-boundable type). Also fixed implementation in kmp_ftn_entry.h – added pointers dereference.
This revision is now accepted and ready to land.Apr 8 2019, 10:32 AM
This revision was automatically updated to reflect the committed changes.