[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea closed this revision.
gtbercea added a comment.

Already covered by https://reviews.llvm.org/D34888


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In https://reviews.llvm.org/D29905#800416, @Hahnfeld wrote:

> In https://reviews.llvm.org/D34888#799576, @gtbercea wrote:
>
> > Does this also include the fixes in the following revision?
> >
> > https://reviews.llvm.org/D29905
>
>
> Sorry, I wasn't aware of this revision and thought that it had long been 
> committed. I just verified that the bug referenced in the summary is also 
> fixed by my patch in https://reviews.llvm.org/D34888. However, I can't 
> comment on whether this patch is still needed. Sorry for the conflicts if 
> yes...
>
> You probably should commit your patches earlier, you currently have 10 
> accepted revisions that have not yet been committed. This will also avoid 
> complicated rebases and so on.


No problem at all! I am trying to commit the rest of the patches. Currently 
waiting on one more patch to get approved that will unlock the rest. I do 
encourage you to review/comment on it: https://reviews.llvm.org/D34784 :)


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D34888#799576, @gtbercea wrote:

> Does this also include the fixes in the following revision?
>
> https://reviews.llvm.org/D29905


Sorry, I wasn't aware of this revision and thought that it had long been 
committed. I just verified that the bug referenced in the summary is also fixed 
by my patch in https://reviews.llvm.org/D34888. However, I can't comment on 
whether this patch is still needed. Sorry for the conflicts if yes...

You probably should commit your patches earlier, you currently have 10 accepted 
revisions that have not yet been committed. This will also avoid complicated 
rebases and so on.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 94990.
gtbercea added a comment.

Fix for loop range.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
+if (Stack.empty())
+  return false;
+
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
1, Check);
+
+if (Stack.size() > 2)
+  for (unsigned I = Stack.size() - 1; I > 0; --I)
+if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 1, Check))
+  return true;
+return false;
+  }
 
-if (SI == SE)
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+if (Stack.size() <= Level)
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+auto StartI = Stack.begin();
+std::advance(StartI, Level);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : MI->second.Components)
-  if (Check(L, MI->second.Kind))
-return true;
-}
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
 return false;
   }
 
@@ -912,9 +924,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level + 1, 
[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
+if (Stack.empty())
+  return false;
+
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 1, Check);
+
+if (Stack.size() > 2)
+  for (unsigned I = Stack.size() - 1; I > 0; --I)
+if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 1, Check))
+  return true;
+return false;
+  }
 
-if (SI == SE)
+  /// Do the check specified in \a Check to all 

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 94985.
gtbercea added a comment.

Refactor code.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
+if (Stack.empty())
+  return false;
+
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
1, Check);
+
+if (Stack.size() > 2)
+  for (unsigned I = Stack.size() - 2; I > 0; --I)
+if (checkMappableExprComponentListsForDeclAtLevel(VD, I, Check))
+  return true;
+return false;
+  }
 
-if (SI == SE)
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+if (Stack.size() <= Level)
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+auto StartI = Stack.begin();
+std::advance(StartI, Level);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : MI->second.Components)
-  if (Check(L, MI->second.Kind))
-return true;
-}
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
 return false;
   }
 
@@ -912,9 +924,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level + 1, 
[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
+if (Stack.empty())
+  return false;
+
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 1, Check);
+
+if (Stack.size() > 2)
+  for (unsigned I = Stack.size() - 2; I > 0; --I)
+if (checkMappableExprComponentListsForDeclAtLevel(VD, I, Check))
+  return true;
+return false;
+  }
 
-if (SI == SE)
+  /// Do the check specified in \a Check to all component lists at 

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 94983.
gtbercea added a comment.

Clean-up.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,36 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
2, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : MI->second.Components)
-  if (Check(L, MI->second.Kind))
-return true;
-}
+for (unsigned I = Stack.size() - 1; I > 0; --I)
+  if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 2, Check))
+return true;
+return false;
+  }
+
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = Stack.begin();
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)(Level + 1))
+  return false;
+std::advance(StartI, Level + 1);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
 return false;
   }
 
@@ -912,9 +923,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,36 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 2, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : 

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:343
+for (unsigned I = Stack.size() - 1; I > 0; --I)
+  if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 2, Check))
+return true;

Are you sure this is correct? If I is 1, I-2 will give you 0x. Please, 
check everything one more time



Comment at: lib/Sema/SemaOpenMP.cpp:355
+   OpenMPClauseKind)> ) {
+// auto StartI = std::next(Stack.begin());
+auto StartI = Stack.begin();

Please, remove this commented code


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 94972.
gtbercea added a comment.

Pass correct levels. Refactor code.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
2, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : MI->second.Components)
-  if (Check(L, MI->second.Kind))
-return true;
-}
+for (unsigned I = Stack.size() - 1; I > 0; --I)
+  if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 2, Check))
+return true;
+return false;
+  }
+
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+// auto StartI = std::next(Stack.begin());
+auto StartI = Stack.begin();
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)(Level + 1))
+  return false;
+std::advance(StartI, Level + 1);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
 return false;
   }
 
@@ -912,9 +924,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,37 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 2, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-   

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 94867.
gtbercea added a comment.

Refactor code.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,36 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
1, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : MI->second.Components)
-  if (Check(L, MI->second.Kind))
-return true;
-}
+for (unsigned I = Stack.size(); I > 0; --I)
+  if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 1, Check))
+return true;
+return false;
+  }
+
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = std::next(Stack.begin());
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)Level)
+  return false;
+std::advance(StartI, Level);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
 return false;
   }
 
@@ -912,9 +923,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -333,25 +333,36 @@
   const llvm::function_ref<
   bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
OpenMPClauseKind)> ) {
-auto SI = Stack.rbegin();
-auto SE = Stack.rend();
-
-if (SI == SE)
+if (Stack.empty())
   return false;
 
-if (CurrentRegionOnly) {
-  SE = std::next(SI);
-} else {
-  ++SI;
-}
+if (CurrentRegionOnly)
+  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 1, Check);
 
-for (; SI != SE; ++SI) {
-  auto MI = SI->MappedExprComponents.find(VD);
-  if (MI != SI->MappedExprComponents.end())
-for (auto  : 

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:358-360
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(

gtbercea wrote:
> ABataev wrote:
> > Could you join these 2 functions into one?
> Since SI and StarI use different types of iterations the functions cannot be 
> merged.
I still believe you can reuse this new code:
```
  bool checkMappableExprComponentListsForDecl(
  ValueDecl *VD, bool CurrentRegionOnly,
  const llvm::function_ref<
  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
   OpenMPClauseKind)> ) {
if (Stack.empty())
  return false;

if (CurrentRegionOnly)
  return checkMappableExprComponentListsForDeclAtLevel(VD, Stack.size() - 
1, Check);

for (unsigned I = Stack.size(); I > 0; --I)
  if (checkMappableExprComponentListsForDeclAtLevel(VD, I - 1, Check))
return true; 
return false;
  }
```
or something like this.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaOpenMP.cpp:358-360
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(

ABataev wrote:
> Could you join these 2 functions into one?
Since SI and StarI use different types of iterations the functions cannot be 
merged.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:358-360
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(

Could you join these 2 functions into one?


Repository:
  rL LLVM

https://reviews.llvm.org/D29905



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

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 93859.
gtbercea added a comment.
Herald added a subscriber: rengolin.

Update test.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -355,6 +355,27 @@
 return false;
   }
 
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = std::next(Stack.begin());
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)Level)
+  return false;
+std::advance(StartI, Level);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
+return false;
+  }
+
   /// Create a new mappable expression component list associated with a given
   /// declaration and initialize it with the provided list of components.
   void addMappableExpressionComponents(
@@ -912,9 +933,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -355,6 +355,27 @@
 return false;
   }
 
+  /// Do the check specified in \a Check to all component lists at a given level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = std::next(Stack.begin());
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)Level)
+  return false;
+std::advance(StartI, Level);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
+return false;
+  }
+
   /// Create a new mappable expression component list associated with a given
   /// declaration and initialize it with the provided list of components.
   void addMappableExpressionComponents(
@@ -912,9 +933,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+

[PATCH] D29905: [OpenMP] Pass argument to device kernel by reference when map is used.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.

When a scalar argument is explicitly mapped, the device kernel needs to be 
passed that argument by reference.

This is a partial fix to this bug 
.

The full fix requires data sharing support which will land in future patches.


Repository:
  rL LLVM

https://reviews.llvm.org/D29905

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,24 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang -DCK30 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+#ifdef CK30
+#include 
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+  #pragma omp target map(tofrom:ParamToKernel)
+  {
+printf("Print ParamToKernel = %d\n", ParamToKernel);
+#pragma omp parallel
+{
+  printf("Do nothing\n");
+}
+  }
+}
+
+// CK30: define void 
@__omp_offloading_802_{{.*}}target_maps_parallel_integer{{.*}}l{{.*}}(i32* 
dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -355,6 +355,27 @@
 return false;
   }
 
+  /// Do the check specified in \a Check to all component lists at a given 
level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = std::next(Stack.begin());
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)Level)
+  return false;
+std::advance(StartI, Level);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
+return false;
+  }
+
   /// Create a new mappable expression component list associated with a given
   /// declaration and initialize it with the provided list of components.
   void addMappableExpressionComponents(
@@ -912,9 +933,8 @@
 bool IsVariableUsedInMapClause = false;
 bool IsVariableAssociatedWithSection = false;
 
-DSAStack->checkMappableExprComponentListsForDecl(
-D, /*CurrentRegionOnly=*/true,
-[&](OMPClauseMappableExprCommon::MappableExprComponentListRef
+DSAStack->checkMappableExprComponentListsForDeclAtLevel(
+D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
 MapExprComponents,
 OpenMPClauseKind WhereFoundClauseKind) {
   // Only the map clause information influences how a variable is


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4756,3 +4756,24 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang -DCK30 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+#ifdef CK30
+#include 
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+  #pragma omp target map(tofrom:ParamToKernel)
+  {
+printf("Print ParamToKernel = %d\n", ParamToKernel);
+#pragma omp parallel
+{
+  printf("Do nothing\n");
+}
+  }
+}
+
+// CK30: define void @__omp_offloading_802_{{.*}}target_maps_parallel_integer{{.*}}l{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -355,6 +355,27 @@
 return false;
   }
 
+  /// Do the check specified in \a Check to all component lists at a given level
+  /// and return true if any issue is found.
+  bool checkMappableExprComponentListsForDeclAtLevel(
+  ValueDecl *VD, unsigned Level,
+  const llvm::function_ref<
+  bool(OMPClauseMappableExprCommon::MappableExprComponentListRef,
+   OpenMPClauseKind)> ) {
+auto StartI = std::next(Stack.begin());
+auto EndI = Stack.end();
+if (std::distance(StartI, EndI) <= (int)Level)
+  return false;
+std::advance(StartI, Level);
+
+auto MI = StartI->MappedExprComponents.find(VD);
+if (MI != StartI->MappedExprComponents.end())
+  for (auto  : MI->second.Components)
+if (Check(L, MI->second.Kind))
+  return true;
+return false;
+  }
+
   ///