[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:14 omp_allocator_handle_t
> | |-EnumConstantDecl 0x11b31e90  col:3 referenced 
> omp_null_allocator 'omp_allocator_handle_t'
> | | `-ImplicitCastExpr 0x11b4b770  'unsigned long' 
> | |   `-ConstantExpr 0x11b31e70  '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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16379
+// UnsignedPointerDiffType avoid truncate.
+AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,

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:14 omp_allocator_handle_t
| |-EnumConstantDecl 0x11b31e90  col:3 referenced 
omp_null_allocator 'omp_allocator_handle_t'
| | `-ImplicitCastExpr 0x11b4b770  'unsigned long' 
| |   `-ConstantExpr 0x11b31e70  'int'
| | |-value: Int 0
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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:
> > > > > > 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16379
+// UnsignedPointerDiffType avoid truncate.
+AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,

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++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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:
> > > > 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16379
+// UnsignedPointerDiffType avoid truncate.
+AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,

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;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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:
> > It must be int type, no?
> unsigned int?
Check the standard, IIRC it says just int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16379
+// UnsignedPointerDiffType avoid truncate.
+AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,

ABataev wrote:
> It must be int type, no?
unsigned int?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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,

It must be int type, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 447372.
jyu2 edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/omp_init_allocator.c
  openmp/runtime/test/api/omp_init_allocator.c

Index: openmp/runtime/test/api/omp_init_allocator.c
===
--- /dev/null
+++ openmp/runtime/test/api/omp_init_allocator.c
@@ -0,0 +1,20 @@
+// RUN: %libomp-compile-and-run
+
+#include 
+#include 
+
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+  omp_destroy_allocator(x_alloc);
+}
+
+int main() {
+  test_allocate_allocator();
+  printf("passed\n");
+}
Index: clang/test/OpenMP/omp_init_allocator.c
===
--- /dev/null
+++ clang/test/OpenMP/omp_init_allocator.c
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -isystem %S/Inputs -emit-llvm -o - -fopenmp \
+// RUN: -fopenmp-version=50 -triple x86_64-unknown-linux-gnu %s | FileCheck %s
+
+typedef enum {
+  omp_atk_sync_hint = 1,
+  omp_atk_alignment = 2
+} omp_alloctrait_key_t;
+typedef unsigned long int uintptr_t;
+typedef uintptr_t omp_uintptr_t;
+typedef struct {
+  omp_alloctrait_key_t key;
+  omp_uintptr_t value;
+} omp_alloctrait_t;
+typedef enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+
+  omp_target_host_mem_alloc = 100,
+  omp_target_shared_mem_alloc = 101,
+  omp_target_device_mem_alloc = 102,
+  KMP_ALLOCATOR_MAX_HANDLE = (18446744073709551615UL)
+} omp_allocator_handle_t;
+typedef enum omp_memspace_handle_t {
+  omp_default_mem_space = 0,
+  omp_large_cap_mem_space = 1,
+  omp_const_mem_space = 2,
+  omp_high_bw_mem_space = 3,
+  omp_low_lat_mem_space = 4,
+
+  omp_target_host_mem_space = 100,
+  omp_target_shared_mem_space = 101,
+  omp_target_device_mem_space = 102,
+  KMP_MEMSPACE_MAX_HANDLE = (18446744073709551615UL)
+} omp_memspace_handle_t;
+
+extern omp_allocator_handle_t omp_init_allocator(omp_memspace_handle_t m,
+ int ntraits,
+ omp_alloctrait_t traits[]);
+//CHECK-LABEL: test_allocate_allocator
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+// CHECK: [[RET:%.+]] = call i64 @omp_init_allocator
+// CHECK-NEXT: store i64 [[RET]], ptr [[X_ALLOC:%x_alloc]]
+// CHECK: [[TMP:%.+]] = load i64, ptr [[X_ALLOC]]
+// CHECK-NEXT: [[CONV:%conv]] = inttoptr i64 [[TMP]] to ptr
+// CHECK-NEXT: call ptr @__kmpc_alloc(i32 %0, i64 4, ptr [[CONV]])
+// CHECK-NOT: trunc
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -16372,8 +16372,12 @@
   ExprResult Allocator = DefaultLvalueConversion(A);
   if (Allocator.isInvalid())
 return nullptr;
-  Allocator = PerformImplicitConversion(Allocator.get(),
-DSAStack->getOMPAllocatorHandleT(),
+  QualType AllocatorHT = DSAStack->getOMPAllocatorHandleT();
+  if (!AllocatorHT->isEnumeralType() && AllocatorHT->isIntegerType())
+// allocator_handler_t is enum type.  For int type convert to
+// UnsignedPointerDiffType avoid truncate.
+AllocatorHT = Context.getUnsignedPointerDiffType();
+  Allocator = PerformImplicitConversion(Allocator.get(), AllocatorHT,
 Sema::AA_Initializing,
 /*AllowExplicit=*/true);
   if (Allocator.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D129872#3672332 , @jyu2 wrote:

> In D129872#3671847 , @ABataev wrote:
>
>> Why does it get truncated if the type must be integer? Probably, something 
>> incorrect in sema.
>
> This is only failed with C.  But not for C++.  So I don't think we have 
> problem for Seam.  I may missing something...
> The different is in reprehensive of enum type between c vs c++:
> For C:
>
> | -EnumDecl 0x1193f838  line:18:14 
> omp_allocator_handle_t |
> | 
>   | -EnumConstantDecl 0x1193f940  col:3 referenced 
> omp_null_allocator 'int'|
> | 
>   | `-ConstantExpr 0x1193f920  'int'  
> |
> | 
>   |   
> | -value: Int 0 |
> | 
>   | `-IntegerLiteral 0x1193f900  'int' 0  
> |
> | 
>   | -EnumConstantDecl 0x1193f9d0  col:7 referenced 
> omp_default_mem_alloc 'int' |
> | 
>   | `-ConstantExpr 0x1193f9b0  'int'  
> |
> | 
>   |   
> | -value: Int 1 |
> | 
>   | `-IntegerLiteral 0x1193f990  'int' 1  
> |
> |
>
> for C++:
>
> | -EnumDecl 0x11b31d88  line:18:14 
> omp_allocator_handle_t |
> | 
>   | -EnumConstantDecl 0x11b31e90  col:3 referenced 
> omp_null_allocator 'omp_allocator_handle_t'|
> | 
>   | `-ImplicitCastExpr 0x11b4b770  'unsigned long'  
>|
> | 
>   | `-ConstantExpr 0x11b31e70  'int'  
>|
> | 
>   |   
>| -value: Int 0 |
> | 
>   | `-IntegerLiteral 0x11b31e50  'int' 0  
>|
> | 
>   | -EnumConstantDecl 0x11b31f20  col:7 referenced 
> omp_default_mem_alloc 'omp_allocator_handle_t' |
> | 
>   | `-ImplicitCastExpr 0x11b4b788  'unsigned long'  
>|
> | 
>   | `-ConstantExpr 0x11b31f00  'int'  
>|
> | 
>   |   
>| -value: Int 1 |
> | 
>   | `-IntegerLiteral 0x11b31ee0  'int' 1  
>|
> |

If the type does not match, we have a problem with casting. Need to add an 
explicit cast to int-like type rather than avoid implicit casts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

In D129872#3671847 , @ABataev wrote:

> Why does it get truncated if the type must be integer? Probably, something 
> incorrect in sema.

This is only failed with C.  But not for C++.  So I don't think we have problem 
for Seam.  I may missing something...
The different is in reprehensive of enum type between c vs c++:
For C:

| -EnumDecl 0x1193f838  line:18:14 omp_allocator_handle_t 
|
|   
| -EnumConstantDecl 0x1193f940  col:3 referenced 
omp_null_allocator 'int'|
|   
| `-ConstantExpr 0x1193f920  'int'  
|
|   
|   
| -value: Int 0 |
|   
| `-IntegerLiteral 0x1193f900  'int' 0  
|
|   
| -EnumConstantDecl 0x1193f9d0  col:7 referenced 
omp_default_mem_alloc 'int' |
|   
| `-ConstantExpr 0x1193f9b0  'int'  
|
|   
|   
| -value: Int 1 |
|   
| `-IntegerLiteral 0x1193f990  'int' 1  
|
|

for C++:

| -EnumDecl 0x11b31d88  line:18:14 omp_allocator_handle_t 
|
|   
| -EnumConstantDecl 0x11b31e90  col:3 referenced 
omp_null_allocator 'omp_allocator_handle_t'|
|   
| `-ImplicitCastExpr 0x11b4b770  'unsigned long'  
   |
|   
| `-ConstantExpr 0x11b31e70  'int'  
   |
|   
|   
   | -value: Int 0 |
|   
| `-IntegerLiteral 0x11b31e50  'int' 0  
   |
|   
| -EnumConstantDecl 0x11b31f20  col:7 referenced 
omp_default_mem_alloc 'omp_allocator_handle_t' |
|   
| `-ImplicitCastExpr 0x11b4b788  'unsigned long'  
   |
|   
| `-ConstantExpr 0x11b31f00  'int'  
   |
|   
|   
   | -value: Int 1 |
|   
| `-IntegerLiteral 0x11b31ee0  'int' 1  
   |
|




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Why does it get truncated if the type must be integer? Probably, something 
incorrect in sema.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 created this revision.
jyu2 added reviewers: ddpagan, ABataev, mikerice.
jyu2 added projects: clang, OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.

The problem is that "alloctor" from omp_init_allocator gets truncated before 
passing to __kmp_alloc.

To fix this, when generate AllocrVal, call to IgnoreImpCasts to skip 'trunc'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/omp_init_allocator.c
  openmp/runtime/test/api/omp_init_allocator.c

Index: openmp/runtime/test/api/omp_init_allocator.c
===
--- /dev/null
+++ openmp/runtime/test/api/omp_init_allocator.c
@@ -0,0 +1,20 @@
+// RUN: %libomp-compile-and-run
+
+#include 
+#include 
+
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+  omp_destroy_allocator(x_alloc);
+}
+
+int main() {
+  test_allocate_allocator();
+  printf("passed\n");
+}
Index: clang/test/OpenMP/omp_init_allocator.c
===
--- /dev/null
+++ clang/test/OpenMP/omp_init_allocator.c
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -isystem %S/Inputs -emit-llvm -o - -fopenmp \
+// RUN: -fopenmp-version=50 -triple x86_64-unknown-linux-gnu %s | FileCheck %s
+
+typedef enum {
+  omp_atk_sync_hint = 1,
+  omp_atk_alignment = 2
+} omp_alloctrait_key_t;
+typedef unsigned long int uintptr_t;
+typedef uintptr_t omp_uintptr_t;
+typedef struct {
+  omp_alloctrait_key_t key;
+  omp_uintptr_t value;
+} omp_alloctrait_t;
+typedef enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+
+  omp_target_host_mem_alloc = 100,
+  omp_target_shared_mem_alloc = 101,
+  omp_target_device_mem_alloc = 102,
+  KMP_ALLOCATOR_MAX_HANDLE = (18446744073709551615UL)
+} omp_allocator_handle_t;
+typedef enum omp_memspace_handle_t {
+  omp_default_mem_space = 0,
+  omp_large_cap_mem_space = 1,
+  omp_const_mem_space = 2,
+  omp_high_bw_mem_space = 3,
+  omp_low_lat_mem_space = 4,
+
+  omp_target_host_mem_space = 100,
+  omp_target_shared_mem_space = 101,
+  omp_target_device_mem_space = 102,
+  KMP_MEMSPACE_MAX_HANDLE = (18446744073709551615UL)
+} omp_memspace_handle_t;
+
+extern omp_allocator_handle_t omp_init_allocator(omp_memspace_handle_t m,
+ int ntraits,
+ omp_alloctrait_t traits[]);
+//CHECK-LABEL: test_allocate_allocator
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+// CHECK: [[RET:%.+]] = call i64 @omp_init_allocator
+// CHECK-NEXT: store i64 [[RET]], ptr [[X_ALLOC:%x_alloc]]
+// CHECK: [[TMP:%.+]] = load i64, ptr [[X_ALLOC]]
+// CHECK-NEXT: [[CONV:%conv]] = inttoptr i64 [[TMP]] to ptr
+// CHECK-NEXT: call ptr @__kmpc_alloc(i32 %0, i64 4, ptr [[CONV]])
+// CHECK-NOT: trunc
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -12114,7 +12114,7 @@
 const Expr *Allocator) {
   llvm::Value *AllocVal;
   if (Allocator) {
-AllocVal = CGF.EmitScalarExpr(Allocator);
+AllocVal = CGF.EmitScalarExpr(Allocator->IgnoreImpCasts());
 // According to the standard, the original allocator type is a enum
 // (integer). Convert to pointer type, if required.
 AllocVal = CGF.EmitScalarConversion(AllocVal, Allocator->getType(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits