[Openmp-commits] [PATCH] D59783: [OpenMP] Implement 5.0 memory management

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

In D59783#4071045 <https://reviews.llvm.org/D59783#4071045>, @ABataev wrote:

> In D59783#4071044 <https://reviews.llvm.org/D59783#4071044>, @tianshilei1992 wrote:
>
>> In D59783#4071039 <https://reviews.llvm.org/D59783#4071039>, @ABataev wrote:
>>
>>> In D59783#4071038 <https://reviews.llvm.org/D59783#4071038>, @tianshilei1992 wrote:
>>>
>>>> In D59783#4071037 <https://reviews.llvm.org/D59783#4071037>, @ABataev wrote:
>>>>
>>>>> In D59783#4071036 <https://reviews.llvm.org/D59783#4071036>, @tianshilei1992 wrote:
>>>>>
>>>>>> In D59783#4071035 <https://reviews.llvm.org/D59783#4071035>, @ABataev wrote:
>>>>>>
>>>>>>> In D59783#4070643 <https://reviews.llvm.org/D59783#4070643>, @tianshilei1992 wrote:
>>>>>>>
>>>>>>>> @ABataev Can we handle the detection of type `omp_allocator_handle_t` in another way? For example, is it possible to get the `TypeDecl *` from the identifier table and use that as the type (we can get the `QualType` from `TypeDecl *`)?
>>>>>>>
>>>>>>> Can you try to fix initializers instead and make them 0UL, 1UL, etc.?
>>>>>>
>>>>>> I tried that. It doesn't work.
>>>>>>
>>>>>>   ➜  cat test_omp_null_allocator.c
>>>>>>   extern int printf(const char *, ...);
>>>>>>   
>>>>>>   typedef enum omp_allocator_handle_t {
>>>>>>     omp_null_allocator = 0UL,
>>>>>>     omp_default_mem_alloc = 1UL,
>>>>>>     omp_large_cap_mem_alloc = 2UL,
>>>>>>     omp_const_mem_alloc = 3UL,
>>>>>>     omp_high_bw_mem_alloc = 4UL,
>>>>>>     omp_low_lat_mem_alloc = 5UL,
>>>>>>     omp_cgroup_mem_alloc = 6UL,
>>>>>>     omp_pteam_mem_alloc = 7UL,
>>>>>>     omp_thread_mem_alloc = 8UL,
>>>>>>     llvm_omp_target_host_mem_alloc = 100UL,
>>>>>>     llvm_omp_target_shared_mem_alloc = 101UL,
>>>>>>     llvm_omp_target_device_mem_alloc = 102UL,
>>>>>>     KMP_ALLOCATOR_MAX_HANDLE = 18446744073709551615UL
>>>>>>   } omp_allocator_handle_t;
>>>>>>   
>>>>>>   int main(int argc, char *argv[]) {
>>>>>>     printf("sizeof(omp_allocator_handle_t)=%zu, sizeof(omp_null_allocator)=%zu\n",
>>>>>>            sizeof(omp_allocator_handle_t), sizeof(omp_null_allocator));
>>>>>>     return 0;
>>>>>>   }
>>>>>>   ➜  clang test_omp_null_allocator.c -o test_omp_null_allocator
>>>>>>   ➜  ./test_omp_null_allocator
>>>>>>   sizeof(omp_allocator_handle_t)=8, sizeof(omp_null_allocator)=4
>>>>>
>>>>> Try ULL instead of UL
>>>>
>>>> But the thing is, this has to work on X86 as well. On 32-bit systems, `void *` is 32-bit, which is a `int`.
>>>
>>> How about explicit vast to uintptr_t, like (uintptr_t) 0, etc.?
>>
>> Not working.
>
> Conditional versions for 32/64 bit systems? With ULL for 64 bit and ul for 32 bits? We'd better to fix the type declaration to make it consistent across compilation units.

Nah, let's forget about portability now, even `0ULL` doesn't work.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D59783