[PATCH] D53636: Do not always request an implicit taskgroup region inside the kmpc_taskloop function

[Sergi Mateo via Phabricator via cfe-commits]

smateo created this revision.
smateo added a reviewer: ABataev.
Herald added a subscriber: cfe-commits.

For the following code:

  int i;
  #pragma omp taskloop
  for (i = 0; i < 100; ++i)
  {}
  
  #pragma omp taskloop nogroup
  for (i = 0; i < 100; ++i)
  {}

Clang emits the following LLVM IR:

  ...
   call void @__kmpc_taskgroup(%struct.ident_t* @0, i32 %0)
   %2 = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @0, i32 %0, i32 1, i64 
80, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, 
%struct.kmp_task_t_with_privates*)* @.omp_task_entry. to i32 (i32, i8*)*))
   ...
   call void @__kmpc_taskloop(%struct.ident_t* @0, i32 %0, i8* %2, i32 1, i64* 
%8, i64* %9, i64 %13, i32 0, i32 0, i64 0, i8* null)
   call void @__kmpc_end_taskgroup(%struct.ident_t* @0, i32 %0)
  
  
   ...
   %15 = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @0, i32 %0, i32 1, 
i64 80, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, 
%struct.kmp_task_t_with_privates.1*)* @.omp_task_entry..2 to i32 (i32, i8*)*))
   ...
   call void @__kmpc_taskloop(%struct.ident_t* @0, i32 %0, i8* %15, i32 1, i64* 
%21, i64* %22, i64 %26, i32 0, i32 0, i64 0, i8* null)

The first set of instructions corresponds to the first taskloop construct. It 
is important to note that the implicit taskgroup region associated with the 
taskloop construct has been materialized in our IR:  the `__kmpc_taskloop` 
occurs inside a taskgroup region. Note also that this taskgroup region does not 
exist in our second taskloop because we are using the `nogroup` clause.

The issue here is the 4th argument of the kmpc_taskloop call, starting from the 
end,  is always a zero. Checking the LLVM OpenMP RT implementation, we see that 
this argument corresponds to the nogroup parameter:

  void __kmpc_taskloop(ident_t *loc, int gtid, kmp_task_t *task, int if_val,
   kmp_uint64 *lb, kmp_uint64 *ub, kmp_int64 st, int 
nogroup,
   int sched, kmp_uint64 grainsize, void *task_dup);

So basically we always tell to the RT to do another taskgroup region. For the 
first taskloop, this means that we create two taskgroup regions. For the second 
example, it means that despite the fact we had a nogroup clause we are going to 
have a taskgroup region, so we unnecessary wait until all descendant tasks have 
been executed.


Repository:
  rC Clang

https://reviews.llvm.org/D53636

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/taskloop_codegen.cpp
  test/OpenMP/taskloop_firstprivate_codegen.cpp
  test/OpenMP/taskloop_lastprivate_codegen.cpp
  test/OpenMP/taskloop_private_codegen.cpp
  test/OpenMP/taskloop_reduction_codegen.cpp
  test/OpenMP/taskloop_simd_codegen.cpp
  test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
  test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
  test/OpenMP/taskloop_simd_private_codegen.cpp
  test/OpenMP/taskloop_simd_reduction_codegen.cpp

Index: test/OpenMP/taskloop_simd_reduction_codegen.cpp
===
--- test/OpenMP/taskloop_simd_reduction_codegen.cpp
+++ test/OpenMP/taskloop_simd_reduction_codegen.cpp
@@ -148,7 +148,7 @@
 // CHECK:[[SUB12:%.*]] = sub nsw i32 [[DIV]], 1
 // CHECK:store i32 [[SUB12]], i32* [[DOTCAPTURE_EXPR_9]],
 // CHECK:[[TMP65:%.*]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i32 1, i64 888, i64 72, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* @{{.+}} to i32 (i32, i8*)*))
-// CHECK:call void @__kmpc_taskloop(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i8* [[TMP65]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 0, i32 0, i64 0, i8* null)
+// CHECK:call void @__kmpc_taskloop(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i8* [[TMP65]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* null)
 // CHECK:call void @__kmpc_end_taskgroup(%struct.ident_t*
 
 // CHECK:ret i32
Index: test/OpenMP/taskloop_simd_private_codegen.cpp
===
--- test/OpenMP/taskloop_simd_private_codegen.cpp
+++ test/OpenMP/taskloop_simd_private_codegen.cpp
@@ -65,7 +65,7 @@
   // LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]](
   // LAMBDA: [[RES:%.+]] = call i8* @__kmpc_omp_task_alloc(%{{[^ ]+}} @{{[^,]+}}, i32 %{{[^,]+}}, i32 1, i64 96, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, %{{[^*]+}}*)* [[TASK_ENTRY:@[^ ]+]] to i32 (i32, i8*)*))
 // LAMBDA: [[PRIVATES:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i{{.+}} 0, i{{.+}} 1
-// LAMBDA: call void @__kmpc_taskloop(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* [[RES]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 0, i32 0, i64 0, i8* null)
+// LAMBDA: call void @__kmpc_taskloop(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* [[RES]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* null)
 // LAMBDA: ret
 #pragma omp taskloop simd private(g, sivar)
   for (int i = 0; i < 10; ++i) {
@@ -101,7 +101,7 @@
   // BLOCKS: define{{.*}} internal{{.*}} vo

[PATCH] D53636: Do not always request an implicit taskgroup region inside the kmpc_taskloop function

[Sergi Mateo via Phabricator via cfe-commits]

smateo added a comment.

Hi Alexey,

Thanks for the prompt review!

I don't have commit access yet, do you mind to commit it for me?

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D53636



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

[PATCH] D56430: Incorrect implicit data-sharing for nested tasks

[Sergi Mateo via Phabricator via cfe-commits]

smateo created this revision.
smateo added a reviewer: ABataev.
smateo added a project: OpenMP.
Herald added a subscriber: cfe-commits.

There is a minor issue in how the implicit data-sharings for nested tasks are 
computed.

For the following example:

  int x;
  #pragma omp task shared(x)
  #pragma omp task
  x++;

We compute an implicit data-sharing of shared for `x` in the second task 
although I think that it should be firstprivate. Below you can find the part of 
the OpenMP spec that covers this example:

- // In a task generating construct, if no default clause is present, a 
variable for which the data-sharing attribute is not determined by the rules 
above and that in the enclosing context is determined to be shared by all 
implicit tasks bound to the current team is shared.//
- //In a task generating construct, if no default clause is present, a variable 
for which the data-sharing attribute is not determined by the rules above is 
firstprivate.//

Since each implicit-task has its own copy of `x`, we shouldn't apply the first 
rule.


Repository:
  rC Clang

https://reviews.llvm.org/D56430

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


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp 
task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared 
private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared 
private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
+
+bool isParallelRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+  return isParallelRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind 
== OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isParallelRegion(I->Directive));
   DVar.CKind =
   (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
   return DVar;


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
=

[PATCH] D56430: Incorrect implicit data-sharing for nested tasks

[Sergi Mateo via Phabricator via cfe-commits]

smateo updated this revision to Diff 180631.
smateo added a comment.

Adding more context


Repository:
  rC Clang

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

https://reviews.llvm.org/D56430

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


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp 
task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared 
private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared 
private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
+
+bool isParallelRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+  return isParallelRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind 
== OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isParallelRegion(I->Directive));
   DVar.CKind =
   (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
   return DVar;


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
+
+bool isParallelRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+  return isParallelRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind == OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isParallelRegion(I->Directive));
   DVar.CKind =
   (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
   return DVar;
_

[PATCH] D56430: Incorrect implicit data-sharing for nested tasks

[Sergi Mateo via Phabricator via cfe-commits]

smateo updated this revision to Diff 180792.
smateo added a comment.

Renaming the `isParallelRegion` function to `isImplicitTaskingRegion`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56430

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


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp 
task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared 
private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared 
private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
+
+bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+  return isImplicitTaskingRegion(DKind) || isOpenMPTaskingDirective(DKind) || 
DKind == OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isImplicitTaskingRegion(I->Directive));
   DVar.CKind =
   (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
   return DVar;


Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
+
+bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+  return isImplicitTaskingRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind == OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isImplicitTaskingRegion(I->Directive));
   DVar.CKind =
   (DVa

[PATCH] D56430: Incorrect implicit data-sharing for nested tasks

[Sergi Mateo via Phabricator via cfe-commits]

smateo updated this revision to Diff 180844.
smateo added a comment.

Done! Thanks,

Sergi


Repository:
  rC Clang

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

https://reviews.llvm.org/D56430

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

Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -8,7 +8,7 @@
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared private here}}
   int a;
 
 public:
@@ -51,6 +51,15 @@
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
@@ -205,6 +214,15 @@
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -676,9 +676,13 @@
   }
 
 };
-bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
- isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+
+bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
+bool isImplicitOrExplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isImplicitTaskingRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind == OMPD_unknown;
 }
 
 } // namespace
@@ -819,7 +823,7 @@
   DVar.CKind = OMPC_firstprivate;
   return DVar;
 }
-  } while (I != E && !isParallelOrTaskRegion(I->Directive));
+  } while (I != E && !isImplicitTaskingRegion(I->Directive));
   DVar.CKind =
   (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
   return DVar;
@@ -1066,7 +1070,7 @@
   if (!isStackEmpty()) {
 iterator I = Iter, E = Stack.back().first.rend();
 Scope *TopScope = nullptr;
-while (I != E && !isParallelOrTaskRegion(I->Directive) &&
+while (I != E && !isImplicitOrExplicitTaskingRegion(I->Directive) &&
!isOpenMPTargetExecutionDirective(I->Directive))
   ++I;
 if (I == E)
@@ -1292,7 +1296,7 @@
   if (FromParent && I != EndI)
 std::advance(I, 1);
   for (; I != EndI; std::advance(I, 1)) {
-if (!DPred(I->Directive) && !isParallelOrTaskRegion(I->Directive))
+if (!DPred(I->Directive) && !isImplicitOrExplicitTaskingRegion(I->Directive))
   continue;
 iterator NewI = I;
 DSAVarData DVar = getDSA(NewI, D);
@@ -1636,7 +1640,7 @@
 auto &&Info = DSAStack->isLoopControlVariable(D);
 if (Info.first ||
 (VD && VD->hasLocalStorage() &&
- isParallelOrTaskRegion(DSAStack->getCurrentDirective())) ||
+ isImplicitOrExplicitTaskingRegion(DSAStack->getCurrentDirective())) ||
 (VD && DSAStack->isForceVarCapturing()))
   return VD ? VD : Info.second;
 DSAStackTy::DSAVarData DVarPrivate =
@@ -2244,7 +2248,7 @@
   // attribute, must have its data-sharing attribute explicitly determined
   // by being listed in a data-sharing attribute clause.
   if (DVar.CKind == OMPC_unknown && Stack->getDefaultDSA() == DSA_none &&
-  isParallelOrTaskRegion(DKind) &&
+  isImplicitOrExplicitTaskingRegion(DKind) &&
   VarsWithInheritedDSA.count(VD) == 0) {
 VarsWithInheritedDSA[VD] = E;
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56430: Incorrect implicit data-sharing for nested tasks

[Sergi Mateo via Phabricator via cfe-commits]

smateo added a comment.

I don't have commit access yet. Would you mind to commit it for me? Thanks!,

Sergi


Repository:
  rC Clang

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

https://reviews.llvm.org/D56430



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