[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:54
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+

cchen wrote:
> Do we really expect `sa[1].f.b` be the same as `[0]`? Shouldn't `sa[1].f.b` 
> be the same as `[0]`?
Yes, looks so, missed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:54
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+

Do we really expect `sa[1].f.b` be the same as `[0]`? Shouldn't `sa[1].f.b` 
be the same as `[0]`?



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:68
+  printf("%d %d %d %4.5f %d\n", sa[1].e, sa[1].f.a, sa[1].f.c.a, sa[1].f.b[1],
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D100673#2716929 , @jdoerfert wrote:

> In D100673#2716916 , @ABataev wrote:
>
>> In D100673#2714087 , @jdoerfert 
>> wrote:
>>
>>> I think this broke GridMini 
>>> (https://github.com/meifeng/GridMini/tree/openmp) :(
>>
>> Hmm, this app does not use mappers. Are you sure that this commit is the 
>> actual cause of the problem? Also, could you provide detailed debug log for 
>> the investigation (with LIBOMPTARGET_DEBUG set)?
>
> I did debug something else and the clang-omp branch failed until I reverted 
> this patch, at least that is what I thought. I will look again as soon as I 
> find the time. For sure it run fine until very recently and we should make 
> sure it continues to do so ;)

Definitely! That's why I'm asking for the debug log for the investigation. It 
requires CUDA but I don't have an access to the machine with cuda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D100673#2716916 , @ABataev wrote:

> In D100673#2714087 , @jdoerfert 
> wrote:
>
>> I think this broke GridMini 
>> (https://github.com/meifeng/GridMini/tree/openmp) :(
>
> Hmm, this app does not use mappers. Are you sure that this commit is the 
> actual cause of the problem? Also, could you provide detailed debug log for 
> the investigation (with LIBOMPTARGET_DEBUG set)?

I did debug something else and the clang-omp branch failed until I reverted 
this branch, at least that is what I thought. I will look again as soon as I 
find the time. For sure it run fine until very recently and we should make sure 
it continues to do so ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D100673#2714087 , @jdoerfert wrote:

> I think this broke GridMini (https://github.com/meifeng/GridMini/tree/openmp) 
> :(

Hmm, this app does not use mappers. Are you sure that this commit is the actual 
cause of the problem? Also, could you provide detailed debug log for the 
investigation (with LIBOMPTARGET_DEBUG set)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D100673#2714087 , @jdoerfert wrote:

> I think this broke GridMini (https://github.com/meifeng/GridMini/tree/openmp) 
> :(

Will investigate this on Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I think this broke GridMini (https://github.com/meifeng/GridMini/tree/openmp) :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG079884225a55: [OPENMP]Fix PR49698: OpenMP declare mapper 
causes segmentation fault. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+
+#pragma omp target map(tofrom : s) firstprivate(p)
+  {
+printf("%d %d %d\n", s.f.a, s.f.c.a,
+   s.f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+s.e = 333;
+s.f.a = 444;
+s.f.c.a = 555;
+s.f.b[1] = 40;
+  }
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  D *sp = 
+
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+#pragma omp target map(tofrom : sp[0]) firstprivate(p)
+  {
+printf("%d %d %d\n", sp[0].f.a, sp[0].f.c.a,
+   sp[0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+sp[0].e = 333;
+sp[0].f.a = 444;
+sp[0].f.c.a = 555;
+sp[0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
@@ -0,0 +1,129 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: 

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp:123-126
+  assert(outer[1].arr[1].arr[0].data1 == 11 &&
+ outer[1].arr[1].arr[0].data2 == 22 &&
+ outer[1].arr[1].arr[1].data1 == 11 &&
+ outer[1].arr[1].arr[1].data2 == 22);

protze.joachim wrote:
> Do these asserts make sense with the assert in line 104?
Ah, just forgot to remove these lines, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Joachim Protze via Phabricator via cfe-commits]

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

The latest changes fix the issue. I tested with x86_64 and nvidia offloading.

The behavior and tests looks good to me, just one comment.




Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp:123-126
+  assert(outer[1].arr[1].arr[0].data1 == 11 &&
+ outer[1].arr[1].arr[0].data2 == 22 &&
+ outer[1].arr[1].arr[1].data1 == 11 &&
+ outer[1].arr[1].arr[1].data2 == 22);

Do these asserts make sense with the assert in line 104?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 338624.
ABataev added a comment.

Fixes and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+
+#pragma omp target map(tofrom : s) firstprivate(p)
+  {
+printf("%d %d %d\n", s.f.a, s.f.c.a,
+   s.f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+s.e = 333;
+s.f.a = 444;
+s.f.c.a = 555;
+s.f.b[1] = 40;
+  }
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  D *sp = 
+
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+#pragma omp target map(tofrom : sp[0]) firstprivate(p)
+  {
+printf("%d %d %d\n", sp[0].f.a, sp[0].f.c.a,
+   sp[0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+sp[0].e = 333;
+sp[0].f.a = 444;
+sp[0].f.c.a = 555;
+sp[0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
@@ -0,0 +1,129 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: 

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D100673#2698168 , @protze.joachim 
wrote:

> I tested the patch and still get wrong results (I modified the 
> `declare_mapper_nested_default_mappers_complex_structure.cpp` test to use my 
> verbose printf from bugzilla):
>
>   Modified Data Hierarchy:
>   Object C (0x623e50, 2) Contents:
>   Object B (0x623e80, 2) Contents:
>   Object A (0x6243f0) Contents:
>   data1 = 6439680  data2 = 0
>   Object A (0x6243f8) Contents:
>   data1 = -784902216  data2 = 11062
>   Object B (0x623e90, 2) Contents:
>   Object A (0x6249e0) Contents:
>   data1 = 6441856  data2 = 0
>   Object A (0x6249e8) Contents:
>   data1 = 6439904  data2 = 0
>   Object C (0x623e60, 2) Contents:
>   Object B (0x623ef0, 2) Contents:
>   Object A (0x624b90) Contents:
>   data1 = 6438736  data2 = 0
>   Object A (0x624b98) Contents:
>   data1 = 6441344  data2 = 0
>   Object B (0x623f00, 2) Contents:
>   Object A (0x624c60) Contents:
>   data1 = 6444032  data2 = 0
>   Object A (0x624c68) Contents:
>   data1 = 6441856  data2 = 0
>
> The values inside the target region look good, but things break when mapping 
> back.
>
> When I remove the `teams distribute for` and only leave `#pragma omp target`, 
> I get a different result and the test mistakenly passes:
>
>   Modified Data Hierarchy:
>   Object C (0x622e40, 2) Contents:
>   Object B (0x622e70, 2) Contents:
>   Object A (0x6233e0) Contents:
>   data1 = 6433120  data2 = 0
>   Object A (0x6233e8) Contents:
>   data1 = 11  data2 = 22
>   Object B (0x622e80, 2) Contents:
>   Object A (0x6239d0) Contents:
>   data1 = 6435792  data2 = 0
>   Object A (0x6239d8) Contents:
>   data1 = 11  data2 = 22
>   Object C (0x622e50, 2) Contents:
>   Object B (0x622ee0, 2) Contents:
>   Object A (0x623b80) Contents:
>   data1 = 6437312  data2 = 0
>   Object A (0x623b88) Contents:
>   data1 = 11  data2 = 22
>   Object B (0x622ef0, 2) Contents:
>   Object A (0x623c50) Contents:
>   data1 = 6437744  data2 = 0
>   Object A (0x623c58) Contents:
>   data1 = 11  data2 = 22
>   Testing for correctness...
>   outer[1].arr[1].arr[1].data2 = 22.
>
> I'm currently only testing x86_64 offloading.

The problem is in transferring from the device. Looks like we need not only to 
invert the list of data in one data transfer block but also invert the list of 
blocks. Trying to implement this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim requested changes to this revision.
protze.joachim added a comment.
This revision now requires changes to proceed.

I tested the patch and still get wrong results (I modified the 
`declare_mapper_nested_default_mappers_complex_structure.cpp` test to use my 
verbose printf from bugzilla):

  Modified Data Hierarchy:
  Object C (0x623e50, 2) Contents:
  Object B (0x623e80, 2) Contents:
  Object A (0x6243f0) Contents:
  data1 = 6439680  data2 = 0
  Object A (0x6243f8) Contents:
  data1 = -784902216  data2 = 11062
  Object B (0x623e90, 2) Contents:
  Object A (0x6249e0) Contents:
  data1 = 6441856  data2 = 0
  Object A (0x6249e8) Contents:
  data1 = 6439904  data2 = 0
  Object C (0x623e60, 2) Contents:
  Object B (0x623ef0, 2) Contents:
  Object A (0x624b90) Contents:
  data1 = 6438736  data2 = 0
  Object A (0x624b98) Contents:
  data1 = 6441344  data2 = 0
  Object B (0x623f00, 2) Contents:
  Object A (0x624c60) Contents:
  data1 = 6444032  data2 = 0
  Object A (0x624c68) Contents:
  data1 = 6441856  data2 = 0

The values inside the target region look good, but things break when mapping 
back.

When I remove the `teams distribute for` and only leave `#pragma omp target`, I 
get a different result and the test mistakenly passes:

  Modified Data Hierarchy:
  Object C (0x622e40, 2) Contents:
  Object B (0x622e70, 2) Contents:
  Object A (0x6233e0) Contents:
  data1 = 6433120  data2 = 0
  Object A (0x6233e8) Contents:
  data1 = 11  data2 = 22
  Object B (0x622e80, 2) Contents:
  Object A (0x6239d0) Contents:
  data1 = 6435792  data2 = 0
  Object A (0x6239d8) Contents:
  data1 = 11  data2 = 22
  Object C (0x622e50, 2) Contents:
  Object B (0x622ee0, 2) Contents:
  Object A (0x623b80) Contents:
  data1 = 6437312  data2 = 0
  Object A (0x623b88) Contents:
  data1 = 11  data2 = 22
  Object B (0x622ef0, 2) Contents:
  Object A (0x623c50) Contents:
  data1 = 6437744  data2 = 0
  Object A (0x623c58) Contents:
  data1 = 11  data2 = 22
  Testing for correctness...
  outer[1].arr[1].arr[1].data2 = 22.

I'm currently only testing x86_64 offloading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG, thanks for the quick fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 338210.
ABataev added a comment.

Added repro from PR49698


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+
+#pragma omp target map(tofrom : s) firstprivate(p)
+  {
+printf("%d %d %d\n", s.f.a, s.f.c.a,
+   s.f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+s.e = 333;
+s.f.a = 444;
+s.f.c.a = 555;
+s.f.b[1] = 40;
+  }
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  D *sp = 
+
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+#pragma omp target map(tofrom : sp[0]) firstprivate(p)
+  {
+printf("%d %d %d\n", sp[0].f.a, sp[0].f.c.a,
+   sp[0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+sp[0].e = 333;
+sp[0].f.a = 444;
+sp[0].f.c.a = 555;
+sp[0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_complex_structure.cpp
@@ -0,0 +1,102 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: 

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D100673#2695694 , @jdoerfert wrote:

> Can we use the reproducer from the bug report, I want an outermost array 
> section with objects that have a declare mapper.

Sure, will add.




Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:7
+
+// XFAIL: clang
+

abhinavgaba wrote:
> Thanks for the fixes, Alexey. Are you planning on handling this case 
> separately?
Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Abhinav Gaba via Phabricator via cfe-commits]

abhinavgaba added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:7
+
+// XFAIL: clang
+

Thanks for the fixes, Alexey. Are you planning on handling this case separately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Can we use the reproducer from the bug report, I want an outermost array 
section with objects that have a declare mapper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, abhinavgaba, jyu2, mikerice.
Herald added subscribers: guansong, yaxunl.
ABataev requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.
Herald added projects: clang, OpenMP.

The implicitly generated mappings for allocation/deallocation in mappers
runtime should be mapped as implicit, also no need to clear member_of
flag to avoid ref counter increment. Also, the ref counter should not be
incremented for the very first element that comes from the mapper
function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100673

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
  
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_var.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+
+#pragma omp target map(tofrom : s) firstprivate(p)
+  {
+printf("%d %d %d\n", s.f.a, s.f.c.a,
+   s.f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+s.e = 333;
+s.f.a = 444;
+s.f.c.a = 555;
+s.f.b[1] = 40;
+  }
+
+  printf("%d %d %d %4.5f %d\n", s.e, s.f.a, s.f.c.a, s.f.b[1],
+ s.f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_ptr_subscript.cpp
@@ -0,0 +1,62 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  D *sp = 
+
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+#pragma omp target map(tofrom : sp[0]) firstprivate(p)
+  {
+printf("%d %d %d\n", sp[0].f.a, sp[0].f.c.a,
+   sp[0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+sp[0].e = 333;
+sp[0].f.a = 444;
+sp[0].f.c.a = 555;
+sp[0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", sp[0].e, sp[0].f.a, sp[0].f.c.a, sp[0].f.b[1],
+ sp[0].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array_subscript.cpp
@@ -0,0 +1,60 @@
+// RUN: