[PATCH] D129872: [clang][OpenMP] Fix runtime crash in the call to __kmp_alloc.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16379
+    // UnsignedPointerDiffType avoid truncate.
+    AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,
----------------
jyu2 wrote:
> ABataev wrote:
> > jyu2 wrote:
> > > ABataev wrote:
> > > > jyu2 wrote:
> > > > > ABataev wrote:
> > > > > > jyu2 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > It must be int type, no?
> > > > > > > unsigned int?
> > > > > > Check the standard, IIRC it says just int.
> > > > > I see that is enum type.
> > > > > 
> > > > > Runtime library definitions: The enum types for omp_allocator_handle_t:
> > > > > 
> > > > > The type omp_allocator_handle_t, which must be an implementation-defined (for C++
> > > > > possibly scoped) enum type with at least the omp_null_allocator enumerator with the
> > > > > value zero and an enumerator for each predefined memory allocator in Table 2.10;
> > > > IIRC, in C enums are always int-based.
> > > That is right.  But not for C++.
> > For C++ need to get the base int type.
> For C++ enum type is unsigned int:  
> since following is defined inside the omp_allocator_handle_t.
>  KMP_ALLOCATOR_MAX_HANDLE = (18446744073709551615UL)  
> 
> But for C it is int.  
> 
> ```
> -EnumDecl 0x11b31d88 <line:18:9, line:33:1> line:18:14 omp_allocator_handle_t
> | |-EnumConstantDecl 0x11b31e90 <line:19:3, col:24> col:3 referenced omp_null_allocator 'omp_allocator_handle_t'
> | | `-ImplicitCastExpr 0x11b4b770 <col:24> 'unsigned long' <IntegralCast>
> | |   `-ConstantExpr 0x11b31e70 <col:24> 'int'
> | |     |-value: Int 0
> ```
> 
> 
I wouldn't rely on this, this is just implementation specific. Instead, need to find corresponding EnumDwcl and extract underlying integer type (getIntegerType). If it is null - the underlying type is int, otherwise use returned type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872