[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen abandoned this revision.
cchen added a comment.

Created a new patch with the support for stride: 
https://reviews.llvm.org/D84192.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D79972#2124108 , @cchen wrote:

> @ABataev , I'm considering emitting an extra dimension for a non-contiguous 
> descriptor to support stride in this patch (stride = 1 in array section is 
> just a special case for computing stride, however, the formula computing 
> stride do not change). Do you think I should do it in this patch?
>
> Computing of stride after support stride in array section:
>
>   int arr[5][5][5];
>   #pragma omp target update to(arr[0:2:2][1:2:1][0:2:2]
>  
>   D0: { offset = 0, count = 1, stride = 4 }   
> // offset, count, dimension size always be 0, 1, 1 for this extra 
> dimension, stride is the unit size
>   D1: { offset = 0, count = 2, stride = 4 * 1 * 2 = 8 }   
>  // stride = unit size * (production of dimension size of D0) * D1.stride = 4 
> * 1 * 2 = 8
>   D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  } 
> // stride = unit size * (production of dimension size of D0, D1) * D2.stride 
> = 4 * 5 * 1 = 20
>   D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 2 = 200 }  // 
> stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride 
> = 4 * 25 * 2 = 200
>
>
> For the case in this patch (stride = 1), we can use the same formula for 
> computing stride with extra dimension:
>
>   int arr[5][5][5];
>   #pragma omp target update to(arr[0:2][1:2][0:2]
>  
>   D0: { offset = 0, count = 1, stride = 4 }   
>// offset, count, dimension size always be 0, 1, 1 for this extra 
> dimension, stride is the unit size
>   D1: { offset = 0, count = 2, stride = 4 * 1 * 1 = 4 }   
>  // stride = unit size * (production of dimension size of D0) * D1.stride = 4 
> * 1 * 1 = 4
>   D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  }// 
> stride = unit size * (production of dimension size of D0, D1) * D2.stride = 4 
> * 5 * 1 = 20
>   D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 1 = 100 } // 
> stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride 
> = 4 * 25 * 1 = 100
>
>
> The extra dimension does not affect the runtime implementation at all since 
> runtime will try to merge inner dimensions if they are contiguous. Take the 
> above case for example (arr[0:2][1:2][0:2]):
>  The product of count and stride for D0 is 4 which is the same as the stride 
> of D1 , therefore, runtime just ignores D0.


You can do this patch. But at first, you need to commit the runtime part of the 
patch that supports it, and the part that introduces stride support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

@ABataev , I'm considering emitting an extra dimension for a non-contiguous 
descriptor to support stride in this patch (stride = 1 in array section is just 
a special case for computing stride, however, the formula computing stride do 
not change). Do you think I should do it in this patch?

Computing of stride after support stride in array section:

  int arr[5][5][5];
  #pragma omp target update to(arr[0:2:2][1:2:1][0:2:2]
  
  D0: { offset = 0, count = 1, stride = 4 } 
  // offset, count, dimension size always be 0, 1, 1 for this extra 
dimension, stride is the unit size
  D1: { offset = 0, count = 2, stride = 4 * 1 * 2 = 8 }
// stride = unit size * (production of dimension size of D0) * D1.stride = 4 * 
1 * 2 = 8
  D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  } // 
stride = unit size * (production of dimension size of D0, D1) * D2.stride = 4 * 
5 * 1 = 20
  D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 2 = 200 }  // 
stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride = 
4 * 25 * 2 = 200

For the case in this patch (stride = 1), we can use the same formula for 
computing stride with extra dimension:

  int arr[5][5][5];
  #pragma omp target update to(arr[0:2][1:2][0:2]
  
  D0: { offset = 0, count = 1, stride = 4 } 
 // offset, count, dimension size always be 0, 1, 1 for this extra 
dimension, stride is the unit size
  D1: { offset = 0, count = 2, stride = 4 * 1 * 1 = 4 }
// stride = unit size * (production of dimension size of D0) * D1.stride = 4 * 
1 * 1 = 4
  D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  }// 
stride = unit size * (production of dimension size of D0, D1) * D2.stride = 4 * 
5 * 1 = 20
  D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 1 = 100 } // 
stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride = 
4 * 25 * 1 = 100

The extra dimension does not affect the runtime implementation at all since 
runtime will try to merge inner dimensions if they are contiguous. Take the 
above case for example (arr[0:2][1:2][0:2]):
The product of count and stride for D0 is 4 which is the same as the stride of 
D1 , therefore, runtime just ignores D0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7046-7049
+/// Signal that the runtime library should use args as an array of
+/// descriptor_dim pointers and use args_size as dims. Used when we have
+/// non-contiguous list items in target update directive
+OMP_MAP_DESCRIPTOR = 0x800,

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > I'm not sure about the value of this flag. If I do recall it correctly, 
> > > this value might be used for something different by XL compiler, for 
> > > example. Maybe use some other value, maybe high bits? It is a kind of 
> > > service flag, not data mapping attribute, so better to move it to high 
> > > bits (the bit before OMP_MAP_MEMBER_OF maybe?).
> > Hi @ABataev, is there any place I can find which value has been used for 
> > lower bits (like 0x800, 0x1000)?
> I rather doubt. You can try to ask @kkwli0 
We are using 0x800.  I think your current choice should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a subscriber: kkwli0.
ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7046-7049
+/// Signal that the runtime library should use args as an array of
+/// descriptor_dim pointers and use args_size as dims. Used when we have
+/// non-contiguous list items in target update directive
+OMP_MAP_DESCRIPTOR = 0x800,

cchen wrote:
> ABataev wrote:
> > I'm not sure about the value of this flag. If I do recall it correctly, 
> > this value might be used for something different by XL compiler, for 
> > example. Maybe use some other value, maybe high bits? It is a kind of 
> > service flag, not data mapping attribute, so better to move it to high bits 
> > (the bit before OMP_MAP_MEMBER_OF maybe?).
> Hi @ABataev, is there any place I can find which value has been used for 
> lower bits (like 0x800, 0x1000)?
I rather doubt. You can try to ask @kkwli0 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7046-7049
+/// Signal that the runtime library should use args as an array of
+/// descriptor_dim pointers and use args_size as dims. Used when we have
+/// non-contiguous list items in target update directive
+OMP_MAP_DESCRIPTOR = 0x800,

ABataev wrote:
> I'm not sure about the value of this flag. If I do recall it correctly, this 
> value might be used for something different by XL compiler, for example. 
> Maybe use some other value, maybe high bits? It is a kind of service flag, 
> not data mapping attribute, so better to move it to high bits (the bit before 
> OMP_MAP_MEMBER_OF maybe?).
Hi @ABataev, is there any place I can find which value has been used for lower 
bits (like 0x800, 0x1000)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 273778.
cchen added a comment.

Rebase and resolve conflictions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr[0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 273747.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16665
 // Record the component - we don't have any declaration associated.
-Components.emplace_back(OASE, nullptr);
+Components.emplace_back(OASE, nullptr, false);
 return RelevantExpr || Visit(E);

`/*IsNonContiguous=*/false`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16685
   // Record the component if haven't found base decl.
-  Components.emplace_back(UO, nullptr);
+  Components.emplace_back(UO, nullptr, false);
 }

`/*IsNonContiguous=*/false`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18470-18473
+MVLI.VarComponents.back().emplace_back(
+OMPClauseMappableExprCommon::MappableComponent(
+SimpleRefExpr, D,
+/*IsNonContiguous=*/false));

`.emplace_back(SimpleRefExpr, D, /*IsNonContiguous=*/false);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 273529.
cchen added a comment.

Pass Info directly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10334-10335
+MCHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes,
+  Info.Dims, Info.Offsets, Info.Counts,
+  Info.Strides);
 

Better to pass `Info` here directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 273517.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16624
+  if (IsPointer && !AllowAnotherPtr)
+SemaRef.Diag(ELoc, diag::err_omp_section_length_undefined) << true;
+  else

cchen wrote:
> ABataev wrote:
> > Better to use integer value as selectors, not boolean.
> The selector for `err_omp_section_length_undefined` is a bool value. (true 
> for unknown bound false for not a array type, so always be true here).
> Do you mean that I need to create a new kind of diagnosis message here and 
> use integer as selectors?
No, it is an integer, starts from `0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16624
+  if (IsPointer && !AllowAnotherPtr)
+SemaRef.Diag(ELoc, diag::err_omp_section_length_undefined) << true;
+  else

ABataev wrote:
> Better to use integer value as selectors, not boolean.
The selector for `err_omp_section_length_undefined` is a bool value. (true for 
unknown bound false for not a array type, so always be true here).
Do you mean that I need to create a new kind of diagnosis message here and use 
integer as selectors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8781
  MappableExprsHandler::MapFlagsArrayTy ,
- CGOpenMPRuntime::TargetDataInfo ) {
+ MappableExprsHandler::MapDimArrayTy ,
+ CGOpenMPRuntime::TargetDataInfo ,

ABataev wrote:
> Do you really need to pass `Dims` here if you have `Dims` data member in 
> `Info` parameter? Why you can't use `Info.Dims` instead?
I think I haven't added Dims in TargetDataInfo atm, I'll add into it and then 
use it via Info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8781
  MappableExprsHandler::MapFlagsArrayTy ,
- CGOpenMPRuntime::TargetDataInfo ) {
+ MappableExprsHandler::MapDimArrayTy ,
+ CGOpenMPRuntime::TargetDataInfo ,

Do you really need to pass `Dims` here if you have `Dims` data member in `Info` 
parameter? Why you can't use `Info.Dims` instead?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8901-8929
+  // Build an array of struct descriptor_dim and then assign it to
+  // offload_args.
+  //
+  // struct descriptor_dim {
+  //  int64_t offset;
+  //  int64_t count;
+  //  int64_t stride

Maybe worth it to outline it into a separate function to reduce code size and 
the complexity of this function? And just call this new function here.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16624
+  if (IsPointer && !AllowAnotherPtr)
+SemaRef.Diag(ELoc, diag::err_omp_section_length_undefined) << true;
+  else

Better to use integer value as selectors, not boolean.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18519-18522
+MVLI.VarComponents.back().emplace_back(
+OMPClauseMappableExprCommon::MappableComponent(
+SimpleRefExpr, D,
+/*IsNonContiguous=*/false));

`.emplace_back(SimpleRefExpr, D, /*IsNonContiguous=*/false);`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18588
 // against other clauses later on.
-OMPClauseMappableExprCommon::MappableComponent MC(SimpleRefExpr, D);
+OMPClauseMappableExprCommon::MappableComponent MC(SimpleRefExpr, D, false);
 DSAStack->addMappableExpressionComponents(

Add a comment for `false` argument with the name of parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 273484.
cchen added a comment.

Fix coding style


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7849-7851
+if ( != &*Components.begin()) {
+  ElementType = ElementType->getPointeeOrArrayElementType();
+}

No need for braces here



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8956
+Info.PointersArray, 0, I);
+Address PAddr(P, C.getTypeAlignInChars(C.VoidPtrTy));
+CGF.Builder.CreateStore(DAddr.getPointer(), PAddr);

`C.getTypeAlignInChars(C.VoidPtrTy)`->`CGF.getPointerAlign()`



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10330-10333
+// Fill up non-contiguous information.
+Info.Offsets = std::move((Offsets));
+Info.Counts = std::move((Counts));
+Info.Strides = std::move((Strides));

Better just to pass `Info.Offsets`, `Info.Counts` and `Info.Strides` as 
arguments to `generateAllInfo()` function and do not create local copies at all.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10583-10586
+// Fill up non-contiguous information.
+Info.Offsets = std::move((Offsets));
+Info.Counts = std::move((Counts));
+Info.Strides = std::move((Strides));

Same, pass the fields as arguments instead.



Comment at: clang/lib/Serialization/ASTReader.cpp:12515
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false));

Still calling an extra constructor here, just `.emplace_back(AssociatedExprPr, 
AssociatedDecl, /*IsNonContiguous=*/false);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12633-12634
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExprPr, AssociatedDecl, IsNonContiguous));
   }

Just `Components.emplace_back(AssociatedExprPr, AssociatedDecl, 
IsNonContiguous);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12684
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExprPr, AssociatedDecl, IsNonContiguous));

`.emplace_back(AssociatedExprPr, AssociatedDecl, IsNonContiguous);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12734
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false));

.`emplace_back(AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12776-12777
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExpr, AssociatedDecl, /*IsNonContiguous*/ false));
   }

`.emplace_back(AssociatedExpr, AssociatedDecl, /*IsNonContiguous*/ false);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12819-12820
 auto *AssociatedDecl = Record.readDeclAs();
-Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExpr, AssociatedDecl, /*IsNonContiguous=*/false));
   }

`.emplace_back(AssociatedExpr, AssociatedDecl, /*IsNonContiguous=*/false));`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2104854 , @RaviNarayanaswamy 
wrote:

> How do you plan to support 
>  #pragma omp target update to (arr[1:2][1:2][0:2], x, b[1:5][0:2])
>  Are you going to split this into 3 updates since your are using the arg 
> fields.


I have added a test basically base on the case in your comment (CK19 in 
target_update_codegen.cpp). Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 272564.
cchen added a comment.

Updated test for clarification


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,304 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2104854 , @RaviNarayanaswamy 
wrote:

> How do you plan to support 
>  #pragma omp target update to (arr[1:2][1:2][0:2], x, b[1:5][0:2])
>  Are you going to split this into 3 updates since your are using the arg 
> fields.


There's only one runtime call for your case. and args will be { descriptor_1, 
x, descriptor_2 }, where descriptor_1 will be { { 1, 2, 80 }, { 1, 2, 20 }, { 
0, 2, 4 } }, descriptor_2 will be { { 1, 5, 16 }, { 0, 2, 4 } }. There's 
analysis in Sema that detecting if the item is non-contiguous or not and 
codegen only generate descriptor for non-contiguous item.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Ravi Narayanaswamy via Phabricator via cfe-commits]

RaviNarayanaswamy added a comment.

How do you plan to support 
#pragma omp target update to (arr[1:2][1:2][0:2], x, b[1:5][0:2])
Are you going to split this into 3 updates since your are using the arg fields.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 271221.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7927
+  CurStrides.push_back(CurStride);
+  DI++;
+}

Use preincrement



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8243
+IsFirstComponentList, L.IsImplicit, CurNonContigInfo,
+/*OverlappedElements=*/llvm::None);
 

No need to add `/*OverlappedElements=*/llvm::None` here, it is default value.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8900-8902
+  if (IsNonContiguous) {
+if (Info.Offsets.empty())
+  return;

Better just to have something like this:
```
if (!IsNonContiguous || Info.Offsets.empty() || Info.NumberOfPtrs == 0)
  return;
...
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 271183.
cchen added a comment.

Fix Int64Ty issue (The bitNum of APInt I used before is 32)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 2 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > cchen wrote:
> > > > ABataev wrote:
> > > > > Create directly as of `CGF.Int64Ty` type.
> > > > Doing this I'll get assertion error in this exact line if on a 32-bits 
> > > > target.
> > > Hmm, why, can you investigate?
> > My comment was not accurate, I've updated it. What I want to convey is that 
> > we can only have `CAT, VAT, or pointer` here, since analysis in Sema has a 
> > restriction for it. (SemaOpenMP line 16623)
> It does not relate to the comments thread but I got it. Anyway, try to 
> investigate why the compiler crashes if you try to cr4eate a constant ща 
> ]СПАюШте64Ен] directly.
I'll investigate it, thanks.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > cchen wrote:
> > > > ABataev wrote:
> > > > > Do you really to pass real offsets here? Can we use pointers instead?
> > > > Do you mean I should set the type of Offset to Expr*?
> > > Currently, you're passing offsets to the runtime. Can we pass pointers 
> > > instead? I mean, for `a[b]` you pass `b` to the runtime, can we pass 
> > > `[b]` instead?
> > Yes, I'm fine either passing index or passing address, though I'm curious 
> > why you're recommending passing address.
> It is going to simplify the codegen. Currently, to get the offset, you need 
> to dig through all the elements of the array section. If, instead, you use 
> the pointers, you would not need to do this and you can rely on something 
> like `CGF.EmitArraySectionLValue()`. At least, I hope so.
After discussed with my colleagues, I think passing relative offset makes more 
sense.

For a 1-dim array, storing the offset as a pointer could work, but it seems 
strange to me to store as a pointer when there are 2+ dimensions with multiple 
disjoint chunks of memory because the pointer can only point to the offset for 
the first chunk. That is, a pointer would refer to an absolute location in a 
single chunk, whereas the offset is relative to the start of any chunk.

For example:

int a[4][4];
#pragma omp target update to(a[1:2][1:2])

This is two disjoint chunks of memory:


XOOX
XOOX


The offset for the outer dimension could be store as a pointer, since there is 
only one instance of that dimension:

Dim1: Offset=[1]

But, the inner dimension is "instantiated" twice, once for each element in the 
outer dimension. So, there are really two absolute pointers, depending on which 
instance (element in the outer dimension) you're talking about:

Dim2: Offset=[1][1]
Dim2: Offset=[2][1]

We could set the policy that the absolute offset would always be expressed as 
the offset in the first instance, but then wouldn't we need to refer to that 
location when computing the offset for all of the other instances? That seems 
unintuitive to me, and potentially complicates the implementation. The relative 
offset makes a lot more senes to me - for a starting point, what relative 
offset is needed for each dimension. The starting point for the outermost 
dimension does require the base address, but all inner dimensions have a 
variable starting pointer based on which element in the outer dimensions you're 
currently looking at.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

cchen wrote:
> ABataev wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > Create directly as of `CGF.Int64Ty` type.
> > > Doing this I'll get assertion error in this exact line if on a 32-bits 
> > > target.
> > Hmm, why, can you investigate?
> My comment was not accurate, I've updated it. What I want to convey is that 
> we can only have `CAT, VAT, or pointer` here, since analysis in Sema has a 
> restriction for it. (SemaOpenMP line 16623)
It does not relate to the comments thread but I got it. Anyway, try to 
investigate why the compiler crashes if you try to cr4eate a constant ща 
]СПАюШте64Ен] directly.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

cchen wrote:
> ABataev wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > Do you really to pass real offsets here? Can we use pointers instead?
> > > Do you mean I should set the type of Offset to Expr*?
> > Currently, you're passing offsets to the runtime. Can we pass pointers 
> > instead? I mean, for `a[b]` you pass `b` to the runtime, can we pass 
> > `[b]` instead?
> Yes, I'm fine either passing index or passing address, though I'm curious why 
> you're recommending passing address.
It is going to simplify the codegen. Currently, to get the offset, you need to 
dig through all the elements of the array section. If, instead, you use the 
pointers, you would not need to do this and you can rely on something like 
`CGF.EmitArraySectionLValue()`. At least, I hope so.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7910
+const Expr *CountExpr = nullptr;
+if (OASE)
+  CountExpr = OASE->getLength();

The check is not required, you already checked that the expression must be 
array section only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 3 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > Create directly as of `CGF.Int64Ty` type.
> > Doing this I'll get assertion error in this exact line if on a 32-bits 
> > target.
> Hmm, why, can you investigate?
My comment was not accurate, I've updated it. What I want to convey is that we 
can only have `CAT, VAT, or pointer` here, since analysis in Sema has a 
restriction for it. (SemaOpenMP line 16623)



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > Do you really to pass real offsets here? Can we use pointers instead?
> > Do you mean I should set the type of Offset to Expr*?
> Currently, you're passing offsets to the runtime. Can we pass pointers 
> instead? I mean, for `a[b]` you pass `b` to the runtime, can we pass `[b]` 
> instead?
Yes, I'm fine either passing index or passing address, though I'm curious why 
you're recommending passing address.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7875-7878
+const auto *OASE = dyn_cast(AssocExpr);
+
+if (!OASE)
+  continue;

ABataev wrote:
> Can we have anything else except for array section here? If not, use just 
> cast. If yes, use continue to simplify complexity:
> ```
> if (!OASE)
>   continue;
> ...
> ```
Not sure about this one, I've added:
```
if (!OASE)
  continue;
...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 270880.
cchen added a comment.

Resolve issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7831-7838
+  llvm::Value *SizeV = nullptr;
+  if (CAT) {
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {
+const Expr *Size = VAT->getSizeExpr();
+SizeV = CGF.EmitScalarExpr(Size);

cchen wrote:
> ABataev wrote:
> > The code for `SizeV` must be under the control of the next `if`:
> > ```
> > if (DimSizes.size() < Components.size() - 1) {
> >  
> > }
> > ```
> I don't think I understand this one. Why do you remove SizeV in the if 
> condition?
This is for `SizeV`. You don't use it if `DimSizes.size() < Components.size() - 
1` is `false`, looks like memory leak.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

cchen wrote:
> ABataev wrote:
> > Create directly as of `CGF.Int64Ty` type.
> Doing this I'll get assertion error in this exact line if on a 32-bits target.
Hmm, why, can you investigate?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

cchen wrote:
> ABataev wrote:
> > Do you really to pass real offsets here? Can we use pointers instead?
> Do you mean I should set the type of Offset to Expr*?
Currently, you're passing offsets to the runtime. Can we pass pointers instead? 
I mean, for `a[b]` you pass `b` to the runtime, can we pass `[b]` instead?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7334
+  OverlappedElements = llvm::None,
+  MapDimArrayTy *const Dims = nullptr) const {
 // The following summarizes what has to be generated for each map and the

Can we encapsulate `Dims` into `StructNonContiguousInfo`?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7837-7838
+Context.getTypeSizeInChars(ElementType).getQuantity();
+  } else if (VAT) {
+ElementType = VAT->getElementType().getTypePtr();
+ElementTypeSize =

If only `CAT` or `VAT` is allowed, then transform this if into:
```
else {
  assert(VAT&& ...);
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7875-7878
+const auto *OASE = dyn_cast(AssocExpr);
+
+if (!OASE)
+  continue;

Can we have anything else except for array section here? If not, use just cast. 
If yes, use continue to simplify complexity:
```
if (!OASE)
  continue;
...
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7917
+if (DI != DimSizes.end()) {
+  CurStride = CGF.Builder.CreateNUWMul(CurStrides.back(), *DI++);
+  CurStrides.push_back(CurStride);

Avoid expressions with some side effects, like `*DI++`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 270536.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 4 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7821
+  Context.getTypeSizeInChars(ElementType).getQuantity();
+} else if (VAT) {
+  ElementType = VAT->getElementType().getTypePtr();

ABataev wrote:
> What if the base is a pointer, not an array?
The `if (ElementType)` condition only push back stride when base is not 
pointer. I'm now allowing one dimension size to be unknown (pointer as base) 
and sema has analysis to check if more than one indirection as base. My last 
codegen test case is for testing pointer as base.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7831-7838
+  llvm::Value *SizeV = nullptr;
+  if (CAT) {
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {
+const Expr *Size = VAT->getSizeExpr();
+SizeV = CGF.EmitScalarExpr(Size);

ABataev wrote:
> The code for `SizeV` must be under the control of the next `if`:
> ```
> if (DimSizes.size() < Components.size() - 1) {
>  
> }
> ```
I don't think I understand this one. Why do you remove SizeV in the if 
condition?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

ABataev wrote:
> Create directly as of `CGF.Int64Ty` type.
Doing this I'll get assertion error in this exact line if on a 32-bits target.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

ABataev wrote:
> Do you really to pass real offsets here? Can we use pointers instead?
Do you mean I should set the type of Offset to Expr*?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7327-7330
+  bool IsNonContiguous = false,
+  MapNonContiguousArrayTy *const Offsets = nullptr,
+  MapNonContiguousArrayTy *const Counts = nullptr,
+  MapNonContiguousArrayTy *const Strides = nullptr) const {

I would prefer to pack these 4 params into a single parameter (a struct). Also, 
can we put `Dims` parameter into the list of the optional parameters?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7805
+  // should be [10, 10] and the first stride is 4 btyes.
+  for (const auto  : Components) {
+const Expr *AssocExpr = Component.getAssociatedExpression();

Expand `auto` here to a real type



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7807
+const Expr *AssocExpr = Component.getAssociatedExpression();
+const auto *OASE = dyn_cast(AssocExpr);
+if (OASE) {

Can we have anything else except for array section here? If not, use just 
`cast`. If yes, use `continue` to simplify complexity:
```
if (!OASE)
  continue;
...
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7821
+  Context.getTypeSizeInChars(ElementType).getQuantity();
+} else if (VAT) {
+  ElementType = VAT->getElementType().getTypePtr();

What if the base is a pointer, not an array?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7831-7838
+  llvm::Value *SizeV = nullptr;
+  if (CAT) {
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {
+const Expr *Size = VAT->getSizeExpr();
+SizeV = CGF.EmitScalarExpr(Size);

The code for `SizeV` must be under the control of the next `if`:
```
if (DimSizes.size() < Components.size() - 1) {
 
}
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

Create directly as of `CGF.Int64Ty` type.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7859
+  // declaration in target update to/from clause.
+  for (const auto  : Components) {
+const Expr *AssocExpr = Component.getAssociatedExpression();

Expand `auto` here to a real type



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7861-7864
+const auto *OASE = dyn_cast(AssocExpr);
+
+if (OASE) {
+  // Offset

Can we have anything else except for array section here? If not, use just 
`cast`. If yes, use `continue` to simplify complexity:
```
if (!OASE)
  continue;
...
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

Do you really to pass real offsets here? Can we use pointers instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 269961.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7046-7049
+/// Signal that the runtime library should use args as an array of
+/// descriptor_dim pointers and use args_size as dims. Used when we have
+/// non-contiguous list items in target update directive
+OMP_MAP_DESCRIPTOR = 0x800,

I'm not sure about the value of this flag. If I do recall it correctly, this 
value might be used for something different by XL compiler, for example. Maybe 
use some other value, maybe high bits? It is a kind of service flag, not data 
mapping attribute, so better to move it to high bits (the bit before 
OMP_MAP_MEMBER_OF maybe?).



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7808-7809
+
+  for (auto CI = Components.begin(), CE = Components.end(); CI != CE;
+   ++CI) {
+const Expr *AssocExpr = CI->getAssociatedExpression();

Use range-based loop, if possible.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7811-7813
+const auto *AE = dyn_cast(AssocExpr);
+const auto *OASE = dyn_cast(AssocExpr);
+if (AE || OASE) {

Do you really need to analyze array subscript expressions here? I though that 
we should analyze only array sections, no?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7870
+  // declaration in target update to/from clause.
+  for (; I != CE; ++I) {
+const Expr *AssocExpr = I->getAssociatedExpression();

Same, try to use range-based loop, if possible.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7875
+
+if (OASE || AE) {
+  // Offset

Same question about array subscript expressions.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8231-8243
+if (L.Components.back().isNonContiguous()) {
+  generateInfoForComponentList(
+  L.MapType, L.MapModifiers, L.Components, CurBasePointers,
+  CurPointers, CurSizes, CurTypes, CurDims, PartialStruct,
+  IsFirstComponentList, L.IsImplicit,
+  /*OverlappedElements=*/llvm::None,
+  /*IsNonContiguous=*/true, , , );

Just `generateInfoForComponentList(
  L.MapType, L.MapModifiers, L.Components, CurBasePointers,
  CurPointers, CurSizes, CurTypes, CurDims, PartialStruct,
  IsFirstComponentList, L.IsImplicit,
  /*OverlappedElements=*/llvm::None,
  L.Components.back().isNonContiguous(), , , 
);`



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8783-8786
+CGOpenMPRuntime::TargetDataInfo , bool IsNonContiguous = false,
+MappableExprsHandler::MapNonContiguousArrayTy *Offsets = nullptr,
+MappableExprsHandler::MapNonContiguousArrayTy *Counts = nullptr,
+MappableExprsHandler::MapNonContiguousArrayTy *Strides = nullptr) {

Can we encapsulate these new data into `CGOpenMPRuntime::TargetDataInfo`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2082017 , @ABataev wrote:

> Do you have a test for mapping of something like `arr[0][:n]`, where the base 
> is an array subscript and the remaining part is an array section?


I'm not having it right now, but it seems like if the base is an array 
subscript and the remaining part is an array section, then this map-item will 
always be contiguous, and will not trigger my code in Codegen. I can still add 
a test for Sema though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 269663.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Do you have a test for mapping of something like `arr[0][:n]`, where the base 
is an array subscript and the remaining part is an array section?




Comment at: clang/include/clang/AST/OpenMPClause.h:4680-4681
+: AssociatedExpressionNonContiguousPr(
+  llvm::PointerIntPair(AssociatedExpression,
+IsNonContiguous)),
   AssociatedDeclaration(

I think you can initialize `AssociatedExpressionNonContiguousPr` using just 
`AssociatedExpressionNonContiguousPr(AssociatedExpression, IsNonContiguous)` 
form, no?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7890
 OpenMPMapClauseKind MapType,
-ArrayRef MapModifiers,
-bool ReturnDevicePointer, bool IsImplicit)
+ArrayRef MapModifiers, bool ReturnDevicePointer,
+bool IsImplicit)

Restore original formatting



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8375-8376
   bool IsFinalArraySection =
-  isFinalArraySectionExpression(I->getAssociatedExpression());
+  isFinalArraySectionExpression(I->getAssociatedExpression()) &&
+  (!IsNonContiguous);
 

Better to convert it to `!IsNonContiguous && 
isFinalArraySectionExpression(I->getAssociatedExpression())`.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8396
+  if (OASE)
+DimSize++;
 

Use prefix form `++DimSize`.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8453
+ /*AddIsTargetParamFlag=*/false,
+ /*IsNonContiguous=*/IsNonContiguous);
   LB = BP;

No need for parameter name comment here, it is required only if the 
`true|false` constants are used



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8508
+ IsCaptureFirstInfo && !RequiresReference,
+ /*IsNonContiguous=*/IsNonContiguous);
 

Same, comment not required



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8563-8577
+  /// Generate the base pointers, section pointers, sizes , map type bits,
+  /// dimension size, offset, count, and strides for the provided map type, map
+  /// modifier, and expression components. \a IsFirstComponent should be set to
+  /// true if the provided set of components is the first associated with a
+  /// capture.
+  void generateInfoForTargetDataComponentList(
+  OpenMPMapClauseKind MapType, ArrayRef 
MapModifiers,

Can we merge the functionality in this new function with the existing ones 
somehow? It is not the best idea to duplicate functionality using copy-paste if 
any.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9353-9355
-/// Emit the arrays used to pass the captures and map information to the
-/// offloading runtime library. If there is no map or capture information,
-/// return nullptr by reference.

Why removed the comment?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9681
+/// return nullptr by reference.
+static void emitTargetDataOffloadingArrays(
+CodeGenFunction ,

Same question as before - can we merge this functionality with the existing 
functions?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18383-18385
 MVLI.VarComponents.back().push_back(
-OMPClauseMappableExprCommon::MappableComponent(SimpleRefExpr, D));
+OMPClauseMappableExprCommon::MappableComponent(SimpleRefExpr, D,
+   false));

Use `.emplace_back(SimpleRefExpr, D, false);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12481-12482
 auto *AssociatedDecl = Record.readDeclAs();
 Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false));
   }

`.emplace_back(AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false);`



Comment at: clang/lib/Serialization/ASTReader.cpp:12599-12600
 auto *AssociatedDecl = Record.readDeclAs();
 Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-AssociatedExpr, AssociatedDecl));
+AssociatedExprPr, AssociatedDecl, IsNonContiguous));
   }

Same, use `emplace_back()`



Comment at: clang/lib/Serialization/ASTReader.cpp:12650
 auto *AssociatedDecl = Record.readDeclAs();
 Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
+AssociatedExprPr, AssociatedDecl, IsNonContiguous));

Same, use `emplace_back()`



Comment at: clang/lib/Serialization/ASTReader.cpp:12700
 auto 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2069435 , @ABataev wrote:

> In D79972#2069366 , @cchen wrote:
>
> > In D79972#2069358 , @ABataev wrote:
> >
> > > In D79972#2069322 , @cchen wrote:
> > >
> > > > In D79972#2068976 , @ABataev 
> > > > wrote:
> > > >
> > > > > Still: Did you think about implementing it in the compiler instead of 
> > > > > the runtime?
> > > >
> > > >
> > > > I'm not sure I understand your question, which part of code are you 
> > > > asking?
> > > >  The main work compiler needs to do is to send the {offset, count, 
> > > > stride} struct to runtime.
> > >
> > >
> > > I mean did you think about calling `__tgt_target_data_update` function in 
> > > a loop in the compiler-generated code instead of putting it into the 
> > > runtime?
> >
> >
> > Oh, I would prefer to call `tgt_target_data_update` once in the compiler 
> > and I'm also doing it now.
>
>
> I was not quite correct. What I mean, is to generate the array with the array 
> section as VLA in the compiler, and fill it in the loop generated by the 
> compiler for non-contiguous sections but not in the runtime?
>  Say, we have the code:
>
>   int arr[3][3]
>   ...
>#pragma omp update to(arr[1:2][1:2]
>  
>
>
> In this case, we're going to transfer the next elements:
>
>   000
>   0xx
>   0xx
>
>
> In the compiler-generated code we emit something like this:
>
>   void *bptr[];
>   void *ptr[];
>   int64 sizes[];
>   int64 maptypes[];
>   for (int i = 0; i < ; ++i) {
> bptr[i] = [1+i][1];
> ptr[i] = [1+i][1];
> sizes[i] = ...;'
> maptypes[i] = ...;
>   }
>   call void @__tgt_target_data_update(i64 -1, i32 , bptr, ptr, sizes, 
> maptypes);
>
>
> With this solution, you won't need to modify the runtime and add a new 
> mapping flag.


For my current implementation, we have discussed in the bi-weekly meeting 
several weeks back, and there was a general consensus that it was an acceptable 
approach.

The major advantage of sending a descriptor to runtime can be elaborated in the 
following example:

  #define N 1
  int a[N][2];
  …
  #pragma amp target update to (a[0:N][0:1])

This would require passing through O(N) entries in the tgt_target_data_update 
call, or 1 entries. The current implementation only require a descriptor 
with 2 entries. I think this could be a real concern -
splitting out the transfers in compiler-generated code results in a list 
containing one entry per non-contiguous chunk (easily hitting scaling issues), 
while the descriptor approach is bounded by the number of dimensions.
That seems like a pretty compelling reason to use the descriptor - it’s much 
more space efficient.

Also, the descriptor idea is very similar to how Cray supported Fortran dope 
vectors for years (we send in a pointer to a dope vector rather than a pointer 
to the data, and a flag to indicate it’s a dope vector, and the runtime library 
handles it as a dope vector).
I think the runtime library changes will not be very extensive or difficult at 
all and we’re very willing to implement the runtime for non-contiguous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 268009.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
@@ -140,6 +144,7 @@
   {
 #pragma omp target update to(s7.x)
   }
+
   return 0;
 }
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D79972#2069366 , @cchen wrote:

> In D79972#2069358 , @ABataev wrote:
>
> > In D79972#2069322 , @cchen wrote:
> >
> > > In D79972#2068976 , @ABataev 
> > > wrote:
> > >
> > > > Still: Did you think about implementing it in the compiler instead of 
> > > > the runtime?
> > >
> > >
> > > I'm not sure I understand your question, which part of code are you 
> > > asking?
> > >  The main work compiler needs to do is to send the {offset, count, 
> > > stride} struct to runtime.
> >
> >
> > I mean did you think about calling `__tgt_target_data_update` function in a 
> > loop in the compiler-generated code instead of putting it into the runtime?
>
>
> Oh, I would prefer to call `tgt_target_data_update` once in the compiler and 
> I'm also doing it now.


I was not quite correct. What I mean, is to generate the array with the array 
section as VLA in the compiler, and fill it in the loop generated by the 
compiler for non-contiguous sections but not in the runtime?
Say, we have the code:

  int arr[3][3]
  ...
   #pragma omp update to(arr[1:2][1:2]

In this case, we're going to transfer the next elements:

  000
  0xx
  0xx

In the compiler-generated code we emit something like this:

  void *bptr[];
  void *ptr[];
  int64 sizes[];
  int64 maptypes[];
  for (int i = 0; i < ; ++i) {
bptr[i] = [1+i][1];
ptr[i] = [1+i][1];
sizes[i] = ...;'
maptypes[i] = ...;
  }
  call void @__tgt_target_data_update(i64 -1, i32 , bptr, ptr, sizes, 
maptypes);

With this solution, you won't need to modify the runtime and add a new mapping 
flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2069358 , @ABataev wrote:

> In D79972#2069322 , @cchen wrote:
>
> > In D79972#2068976 , @ABataev wrote:
> >
> > > Still: Did you think about implementing it in the compiler instead of the 
> > > runtime?
> >
> >
> > I'm not sure I understand your question, which part of code are you asking?
> >  The main work compiler needs to do is to send the {offset, count, stride} 
> > struct to runtime.
>
>
> I mean did you think about calling `__tgt_target_data_update` function in a 
> loop in the compiler-generated code instead of putting it into the runtime?


Oh, I would prefer to call `tgt_target_data_update` once in the compiler and 
I'm also doing it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D79972#2069322 , @cchen wrote:

> In D79972#2068976 , @ABataev wrote:
>
> > Still: Did you think about implementing it in the compiler instead of the 
> > runtime?
>
>
> I'm not sure I understand your question, which part of code are you asking?
>  The main work compiler needs to do is to send the {offset, count, stride} 
> struct to runtime.


I mean did you think about calling `__tgt_target_data_update` function in a 
loop in the compiler-generated code instead of putting it into the runtime?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2068976 , @ABataev wrote:

> Still: Did you think about implementing it in the compiler instead of the 
> runtime?


I'm not sure I understand your question, which part of code are you asking?
The main work compiler needs to do is to send the {offset, count, stride} 
struct to runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Still: Did you think about implementing it in the compiler instead of the 
runtime?




Comment at: clang/include/clang/AST/OpenMPClause.h:4676-4677
 explicit MappableComponent() = default;
-explicit MappableComponent(Expr *AssociatedExpression,
+explicit MappableComponent(llvm::PointerIntPair
+   AssociatedExpressionNonContiguousPr,
ValueDecl *AssociatedDeclaration)

I would suggest to pass `Expr *` and `bool` as separate parameters here rather 
than as `PointerIntPair` 



Comment at: clang/include/clang/AST/OpenMPClause.h:4690
+
+bool getNonContiguous() const {
+  return AssociatedExpressionNonContiguousPr.getInt();

`isNonContiguous()`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16356
   OMPClauseMappableExprCommon::MappableExprComponentList 
+  bool IsNonContiguous;
   bool NoDiagnose = false;

Add default initializer



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16540-16544
+  if (IsPointer && !AllowAnotherPtr) {
+SemaRef.Diag(ELoc, diag::err_omp_section_length_undefined) << true;
+  } else {
+IsNonContiguous = true;
+  }

Remove braces here, they are not needed.



Comment at: clang/lib/Serialization/ASTWriter.cpp:6575
 Record.AddStmt(M.getAssociatedExpression());
+Record.push_back(M.getNonContiguous());
 Record.AddDeclRef(M.getAssociatedDeclaration());

There is a member function `writeBool`



Comment at: clang/lib/Serialization/ASTWriter.cpp:6600
 Record.AddStmt(M.getAssociatedExpression());
+Record.push_back(M.getNonContiguous());
 Record.AddDeclRef(M.getAssociatedDeclaration());

Same, use `writeBool`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 267725.
cchen added a comment.

- Use PointerIntPair to pass non-contiguous information in AST
- Error out in Sema if we don't have enough size information for cases 
involving pointers
- Allows `*arr[N][M]` since we don't need size information for the last 
dimension
- Add more test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+  double marr[10][5][10];
+#pragma omp target update to(marr[0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
@@ -140,6 +144,7 @@
   {
 #pragma omp target update to(s7.x)
   }
+
   return 0;
 }
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr[0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16537
 AllowWholeSizeArraySection = false;
+} else if (DKind == OMPD_target_update &&
+   SemaRef.getLangOpts().OpenMP >= 50) {

cchen wrote:
> ABataev wrote:
> > cchen wrote:
> > > @ABataev , I guess you're saying the condition should be 
> > > `!AllowWholeSizeArraySection && DKind == OMPD_target_update && 
> > > SemaRef.getLangOpts().OpenMP >= 50`?
> > No, what I want is to try to simplify the code. I see now why do you need 
> > this flag. I'm just thinking can we avoid adding this flag to the clause 
> > and save some mem space?
> But we also don't want to do the analysis in codegen I guess? Also, if we 
> emit non-contiguous runtime for every target update call, we need to change 
> tons of stuff (tons of lit tests, runtime implementation, etc...).
Maybe make it a part of `MappableComponent`, if possible, and put it into 
`PointerIntPair AssociatedExpression;`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 2 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8397-8400
+  if (OASE || OAShE ||
+  dyn_cast(I->getAssociatedExpression())) {
+DimSize++;
+  }

ABataev wrote:
> Do you really need to count `DimSize` for array shaping operators and array 
> subscript expressions? I don't see tests for it.
You're right, I don't need to count `DimSize` for array shaping and array 
subscript.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16537
 AllowWholeSizeArraySection = false;
+} else if (DKind == OMPD_target_update &&
+   SemaRef.getLangOpts().OpenMP >= 50) {

ABataev wrote:
> cchen wrote:
> > @ABataev , I guess you're saying the condition should be 
> > `!AllowWholeSizeArraySection && DKind == OMPD_target_update && 
> > SemaRef.getLangOpts().OpenMP >= 50`?
> No, what I want is to try to simplify the code. I see now why do you need 
> this flag. I'm just thinking can we avoid adding this flag to the clause and 
> save some mem space?
But we also don't want to do the analysis in codegen I guess? Also, if we emit 
non-contiguous runtime for every target update call, we need to change tons of 
stuff (tons of lit tests, runtime implementation, etc...).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 266921.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,142 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// SIMD-ONLY19-NOT: {{__kmpc|__tgt}}
+#ifdef CK19
+
+// CK19: [[STRUCT_DESCRIPTOR:%.+]]  = type { i64, i64, i64 }
+
+// CK19: [[MSIZE:@.+]] = {{.+}}constant [1 x i64] [i64 3]
+// CK19: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 2081]
+
+// CK19-LABEL: _Z3foo
+void foo(int arg) {
+  int arr[3][4][5];
+
+  // CK19: [[DIMS:%.+]] = alloca [3 x [[STRUCT_DESCRIPTOR]]],
+  // CK19: [[ARRAY_IDX:%.+]] = getelementptr inbounds [3 x [4 x [5 x i32]]], [3 x [4 x [5 x i32]]]* [[ARR:%.+]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_DECAY:%.+]] = getelementptr inbounds [4 x [5 x i32]], [4 x [5 x i32]]* [[ARRAY_IDX]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_1:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_DECAY]], {{.+}}
+  // CK19: [[ARRAY_DECAY_2:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_IDX_1]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_3:%.+]] = getelementptr inbounds {{.+}}, {{.+}}* [[ARRAY_DECAY_2]], {{.+}} 1
+  // CK19: [[LEN:%.+]] = sub nuw i64 4, [[ARG_ADDR:%.+]]
+  // CK19: [[BP0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[BP:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[P0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[P:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[DIM_1:%.+]] = getelementptr inbounds [3 x [[STRUCT_DESCRIPTOR]]], [3 x [[STRUCT_DESCRIPTOR]]]* [[DIMS]], {{.+}} 0, {{.+}} 0
+  // CK19: [[OFFSET:%.+]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]], [[STRUCT_DESCRIPTOR]]* [[DIM_1]], {{.+}} 0, {{.+}} 0
+  // CK19: store i64 0, i64* [[OFFSET]],
+  // CK19: [[COUNT:%.+]] = getelementptr inbounds 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Did you think about implementing it in the compiler instead of the runtime?




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8397-8400
+  if (OASE || OAShE ||
+  dyn_cast(I->getAssociatedExpression())) {
+DimSize++;
+  }

Do you really need to count `DimSize` for array shaping operators and array 
subscript expressions? I don't see tests for it.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16537
 AllowWholeSizeArraySection = false;
+} else if (DKind == OMPD_target_update &&
+   SemaRef.getLangOpts().OpenMP >= 50) {

cchen wrote:
> @ABataev , I guess you're saying the condition should be 
> `!AllowWholeSizeArraySection && DKind == OMPD_target_update && 
> SemaRef.getLangOpts().OpenMP >= 50`?
No, what I want is to try to simplify the code. I see now why do you need this 
flag. I'm just thinking can we avoid adding this flag to the clause and save 
some mem space?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16537
 AllowWholeSizeArraySection = false;
+} else if (DKind == OMPD_target_update &&
+   SemaRef.getLangOpts().OpenMP >= 50) {

@ABataev , I guess you're saying the condition should be 
`!AllowWholeSizeArraySection && DKind == OMPD_target_update && 
SemaRef.getLangOpts().OpenMP >= 50`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2058608 , @ABataev wrote:

> In D79972#2058555 , @cchen wrote:
>
> > In D79972#2058516 , @ABataev wrote:
> >
> > > Is my guess correct that for OpenMP >= 50 for target update directive we 
> > > always emit `possibly non-continuous` runtime calls?
> >
> >
> > My intent is to emit `possibly non-contiguous` runtime calls only if the 
> > analysis in Sema set the IsNonContiguous flag to true.
>
>
> But this analysis only checks for the directive and the version,nothing else.


The context of the checks for the directive and version:

  bool NotWhole =
checkArrayExpressionDoesNotReferToWholeSize(SemaRef, OASE, CurType);
  bool NotUnity =
checkArrayExpressionDoesNotReferToUnitySize(SemaRef, OASE, CurType);
  
  if (AllowWholeSizeArraySection) {
// Any array section is currently allowed. Allowing a whole size array
// section implies allowing a unity array section as well.
//
// If this array section refers to the whole dimension we can still
// accept other array sections before this one, except if the base is a
// pointer. Otherwise, only unitary sections are accepted.
if (NotWhole || IsPointer)
  AllowWholeSizeArraySection = false;
  } else if (DKind == OMPD_target_update &&
 SemaRef.getLangOpts().OpenMP >= 50) {
IsNonContiguousRef = true;
  } else if (AllowUnitySizeArraySection && NotUnity) {
// A unity or whole array section is not allowed and that is not
// compatible with the properties of the current array section.
SemaRef.Diag(
  ELoc, diag::err_array_section_does_not_specify_contiguous_storage)
  << OASE->getSourceRange();
return false;
  }

The original analysis checks for non-contiguous by finding if there is more 
than one "array-section" expression with length greater than one. Therefore, I 
added my check there to allow more than one array-section with length greater 
than one by depending on the existing analysis (and also set IsNonContiguous to 
true so that we can pass it to codegen rather than doing analysis in codegen). 
This change allows me to pass all the existing lit test but still emit the 
"non-contiguous" runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D79972#2058555 , @cchen wrote:

> In D79972#2058516 , @ABataev wrote:
>
> > Is my guess correct that for OpenMP >= 50 for target update directive we 
> > always emit `possibly non-continuous` runtime calls?
>
>
> My intent is to emit `possibly non-contiguous` runtime calls only if the 
> analysis in Sema set the IsNonContiguous flag to true.


But this analysis only checks for the directive and the version,nothing else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2058516 , @ABataev wrote:

> Is my guess correct that for OpenMP >= 50 for target update directive we 
> always emit `possibly non-continuous` runtime calls?


My intent is to emit `possibly non-contiguous` runtime calls only if the 
analysis in Sema set the IsNonContiguous flag to true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:5315
+  private llvm::TrailingObjects<
+  OMPMapClause, Expr *, ValueDecl *, bool, unsigned,
+  OMPClauseMappableExprCommon::MappableComponent> {

ABataev wrote:
> Why do you need this bool flag? Seems to me, it is set to `true` always if 
> `OpenMP >= 50 && Directive == OMPD_target_update`. Could check it during the 
> codegen rather than introduce this new extra data here?
You're right, I shouldn't add bool here since we only need it in OMPToClause 
and OMPFromClause. I was adding it since I'm assuming they should have the same 
type for the inherited TrailingObject.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Is my guess correct that for OpenMP >= 50 for target update directive we always 
emit `possibly non-continuous` runtime calls?




Comment at: clang/include/clang/AST/OpenMPClause.h:5315
+  private llvm::TrailingObjects<
+  OMPMapClause, Expr *, ValueDecl *, bool, unsigned,
+  OMPClauseMappableExprCommon::MappableComponent> {

Why do you need this bool flag? Seems to me, it is set to `true` always if 
`OpenMP >= 50 && Directive == OMPD_target_update`. Could check it during the 
codegen rather than introduce this new extra data here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D79972#2049838 , @ABataev wrote:

> How are you going to pass this non-contiguous data in the runtime? Are you 
> going to map it in a loop or convert this non-contiguous data into the 
> contiguous and map it as a contiguous chunk of data? Your presentation 
> provides interface only interface changes but has nothing about 
> implementation in the runtime.


Hi Alexey, thanks for asking. The runtime implementation I'm thinking of is to 
convert the non-contiguous data into several chunks of contiguous.

For example:

int arr[3][3][3];

#pragma omp target update to (arr[1:2][1:2][0:2])

We can visualize the noncontiguous data as below (X is the data we want to 
transfer, O is the data want don't bother with):

Dim 0 = {Offset: 0, Count: 1, Stride: 4bytes (int)}
XXO

Dim 1 = {Offset: 1, Count: 2, Stride: 12bytes (4 * 3 - since Dim 0 has 3 
elements)
OOO
XXO
XXO

Dim 2 = {Offset: 1, Count: 2, Stride: 36 bytes (12 * 3 since Dim 1 has 3 
elements)
OOO
OOO
OOO
\
OOO
XXO
XXO
\
OOO
XXO
XXO

For the visualization, we know that we want to transfer 4 contiguous chunks and 
the runtime code could be something similar to:

  // we expect this loop to transfer 4 contiguous chunks:
  // arr[1][1][0:2]
  // arr[1][2][0:2]
  // arr[2][1][0:2]
  // arr[2][2][0:2]
  for (int i = Dim[2].offset; i < Dim[2].count; i++) {
for (int j = Dim[1].offset; j < Dim[1].count; j++) {
  ptr = bast_ptr + Dim[2].stride * i + Dim[1].stride * j + Dim[2].stride * 
Dim[0].offset;
  size = Dim[0].count * Dim[0].stride;  // we can hoist it I think
  transfer(ptr, size, /*flag or some other stuff...*/);
}
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

How are you going to pass this non-contiguous data in the runtime? Are you 
going to map it in a loop or convert this non-contiguous data into the 
contiguous and map it as a contiguous chunk of data? Your presentation provides 
interface only interface changes but has nothing about implementation in the 
runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 265562.
cchen added a comment.
Herald added a subscriber: sstefan1.

Remove redundant code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,142 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// SIMD-ONLY19-NOT: {{__kmpc|__tgt}}
+#ifdef CK19
+
+// CK19: [[STRUCT_DESCRIPTOR:%.+]]  = type { i64, i64, i64 }
+
+// CK19: [[MSIZE:@.+]] = {{.+}}constant [1 x i64] [i64 3]
+// CK19: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 2081]
+
+// CK19-LABEL: _Z3foo
+void foo(int arg) {
+  int arr[3][4][5];
+
+  // CK19: [[DIMS:%.+]] = alloca [3 x [[STRUCT_DESCRIPTOR]]],
+  // CK19: [[ARRAY_IDX:%.+]] = getelementptr inbounds [3 x [4 x [5 x i32]]], [3 x [4 x [5 x i32]]]* [[ARR:%.+]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_DECAY:%.+]] = getelementptr inbounds [4 x [5 x i32]], [4 x [5 x i32]]* [[ARRAY_IDX]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_1:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_DECAY]], {{.+}}
+  // CK19: [[ARRAY_DECAY_2:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_IDX_1]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_3:%.+]] = getelementptr inbounds {{.+}}, {{.+}}* [[ARRAY_DECAY_2]], {{.+}} 1
+  // CK19: [[LEN:%.+]] = sub nuw i64 4, [[ARG_ADDR:%.+]]
+  // CK19: [[BP0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[BP:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[P0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[P:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[DIM_1:%.+]] = getelementptr inbounds [3 x [[STRUCT_DESCRIPTOR]]], [3 x [[STRUCT_DESCRIPTOR]]]* [[DIMS]], {{.+}} 0, {{.+}} 0
+  // CK19: [[OFFSET:%.+]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]], [[STRUCT_DESCRIPTOR]]* [[DIM_1]], {{.+}} 0, {{.+}} 0
+  // CK19: store i64 0, i64* [[OFFSET]],
+  // CK19: [[COUNT:%.+]] = 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 264266.
cchen added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,142 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// SIMD-ONLY19-NOT: {{__kmpc|__tgt}}
+#ifdef CK19
+
+// CK19: [[STRUCT_DESCRIPTOR:%.+]]  = type { i64, i64, i64 }
+
+// CK19: [[MSIZE:@.+]] = {{.+}}constant [1 x i64] [i64 3]
+// CK19: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 2081]
+
+// CK19-LABEL: _Z3foo
+void foo(int arg) {
+  int arr[3][4][5];
+
+  // CK19: [[DIMS:%.+]] = alloca [3 x [[STRUCT_DESCRIPTOR]]],
+  // CK19: [[ARRAY_IDX:%.+]] = getelementptr inbounds [3 x [4 x [5 x i32]]], [3 x [4 x [5 x i32]]]* [[ARR:%.+]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_DECAY:%.+]] = getelementptr inbounds [4 x [5 x i32]], [4 x [5 x i32]]* [[ARRAY_IDX]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_1:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_DECAY]], {{.+}}
+  // CK19: [[ARRAY_DECAY_2:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_IDX_1]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_3:%.+]] = getelementptr inbounds {{.+}}, {{.+}}* [[ARRAY_DECAY_2]], {{.+}} 1
+  // CK19: [[LEN:%.+]] = sub nuw i64 4, [[ARG_ADDR:%.+]]
+  // CK19: [[BP0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[BP:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[P0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[P:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[DIM_1:%.+]] = getelementptr inbounds [3 x [[STRUCT_DESCRIPTOR]]], [3 x [[STRUCT_DESCRIPTOR]]]* [[DIMS]], {{.+}} 0, {{.+}} 0
+  // CK19: [[OFFSET:%.+]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]], [[STRUCT_DESCRIPTOR]]* [[DIM_1]], {{.+}} 0, {{.+}} 0
+  // CK19: store i64 0, i64* [[OFFSET]],
+  // CK19: [[COUNT:%.+]] = getelementptr 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, guansong, yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

In order not to modify the `tgt_target_data_update` information but still be
able to pass the extra information for non-contiguous map item (offset,
count, and stride for each dimension), this patch overload `arg` when
the maptype is set as `OMP_MAP_DESCRIPTOR`. The origin `arg` is for
passing the pointer information, however, the overloaded `arg` is an
array of descriptor_dim:

struct descriptor_dim {

  int64_t offset;
  int64_t count;
  int64_t stride

};

and the array size is the same as dimension size. In addition, since we
have count and stride information in descriptor_dim, we can replace/overload the
`arg_size` parameter by using dimension size.

More details can be found here: 
https://github.com/chichunchen/openmp-50-design/blob/master/target_update_noncontiguous.pptx


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,142 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// SIMD-ONLY19-NOT: {{__kmpc|__tgt}}
+#ifdef CK19
+
+// CK19: [[STRUCT_DESCRIPTOR:%.+]]  = type { i64, i64, i64 }
+
+// CK19: [[MSIZE:@.+]] = {{.+}}constant [1 x i64] [i64 3]
+// CK19: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 2081]
+
+// CK19-LABEL: _Z3foo
+void foo(int arg) {
+  int arr[3][4][5];
+
+  // CK19: [[DIMS:%.+]] = alloca [3 x [[STRUCT_DESCRIPTOR]]],
+  // CK19: [[ARRAY_IDX:%.+]] = getelementptr inbounds [3 x [4 x [5 x i32]]], [3 x [4 x [5 x i32]]]* [[ARR:%.+]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_DECAY:%.+]] = getelementptr inbounds [4 x [5 x i32]], [4 x [5 x i32]]* [[ARRAY_IDX]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_1:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_DECAY]], {{.+}}
+