[Flang][OpenMP] Fixed semantic error when list item with a private da…

[kaviya2510]

…ta-sharing clause used in 'allocate' clause of 'taskgroup' construct.

Issue: A list item with a private data-sharing clause used in 'allocate' clause of 'taskgroup' construct throws an error "The ALLOCATE clause requires that 'x' must be listed in a private data-sharing attribute clause on the same directive"

Fix: As omp_taskgroup doesn't accept data sharing clauses, the list item in the allocate clause should accept the data sharing clause of list items from the enclosing directive.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: None (kaviya2510)

Changes

…ta-sharing clause used in 'allocate' clause of 'taskgroup' construct.

Issue: A list item with a private data-sharing clause used in 'allocate' clause of 'taskgroup' construct throws an error "The ALLOCATE clause requires that 'x' must be listed in a private data-sharing attribute clause on the same directive"

Fix: As omp_taskgroup doesn't accept data sharing clauses, the list item in the allocate clause should accept the data sharing clause of list items from the enclosing directive.

---

Full diff: https://github.com/llvm/llvm-project/pull/109775.diff

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-directives.cpp (+10-2)
• (added) flang/test/Semantics/OpenMP/omp_taskgroup_allocate.f90 (+12)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 17567a555db326..6de37f5235e098 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1540,8 +1540,16 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPBlockConstruct &x) {
   }
   if (beginDir.v == llvm::omp::Directive::OMPD_master)
     IssueNonConformanceWarning(beginDir.v, beginDir.source);
-  ClearDataSharingAttributeObjects();
-  ClearPrivateDataSharingAttributeObjects();
+
+  // The omp_taskgroup directive doesn't have its own data-sharing clauses.
+  // It inherits data-sharing attributes from the surrounding context.
+  // Therefore, don't clear the data-sharing attributes if it's an omp taskgroup
+  if (beginDir.v != llvm::omp::Directive::OMPD_taskgroup)
+  {
+    ClearDataSharingAttributeObjects();
+    ClearPrivateDataSharingAttributeObjects();
+  }
+
   ClearAllocateNames();
   return true;
 }
diff --git a/flang/test/Semantics/OpenMP/omp_taskgroup_allocate.f90 b/flang/test/Semantics/OpenMP/omp_taskgroup_allocate.f90
new file mode 100644
index 00000000000000..36bb713f3a13fa
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/omp_taskgroup_allocate.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -fopenmp -fsyntax-only %s
+
+! Verify that a list item with a private data-sharing clause used in the 'allocate' clause of 'taskgroup'
+! causes no semantic errors.
+
+subroutine omp_allocate_taskgroup
+   integer :: x
+   !$omp parallel private(x)
+   !$omp taskgroup allocate(x)
+   !$omp end taskgroup
+   !$omp end parallel
+end subroutine

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[kiranchandramohan]

Is this only for taskgroup? Are there other constructs like this?

[kaviya2510]

Thanks for the comments.
yes, This is only for taskgroup.
All other construct which uses allocate clause other than taskgroup and allocators accepts datasharing attribute clauses as per Openmp 5.2 standard.
This case is already handled in allocators directive and this issue is only observed with allocate cause of taskgroup construct.

[kaviya2510]

Hi @kiranchandramohan,
Hope you have read my comments. Please do let me know if it needs any further changes.

[kiranchandramohan]

Can you run with test_errors.py to ensure that no Error messages are generated?

[kaviya2510]

Sure. I have tested it with test_error.py and no error are generated.
I haven't used test_error.py for validating this testcase by referring the below comment https://github.com/llvm/llvm-project/pull/109265/files#r1767132309.

[tblah]

Please could you add a reference to the relevant clause of the standard to the commit description and/or a code comment. I am finding this difficult to find.

[kparzysz]

Please could you add a reference to the relevant clause of the standard to the commit description and/or a code comment. I am finding this difficult to find.

Yeah, I'm pretty sure this isn't legal. The ALLOCATE clause must be on the same directive that has the privatizing clause. It specifies how to allocate memory for the private copy, so adding it in a nested construct is questionable.

[kaviya2510]

he ALLOCATE clause must be on the same directive that has the privatizing clause. It specifies how to allocate memory for the private copy, so adding it in a nested construct is questionable.

Please could you add a reference to the relevant clause of the standard to the commit description and/or a code comment. I am finding this difficult to find.

Hi @tblah,
Thanks for the comments.
Taskgroup
Kindly refer the above link for taskgroup directive.
According to the OpenMP standard, the allocate clause must be used on the same directive that has the private data-sharing clause. However, in this case, the taskgroup directive only supports the allocate and task_reduction clauses, making it impossible to specify the data-sharing attribute of a variable within the taskgroup directive.

[kaviya2510]

Yeah, I'm pretty sure this isn't legal. The ALLOCATE clause must be on the same directive that has the privatizing clause. It specifies how to allocate memory for the private copy, so adding it in a nested construct is questionable.

Hi @kparzysz
Yeah, I agree with your point. But taskgroup directive is not allowing data_sharing_clauses in it.
Kindly refer the below comment #109775 (comment) for more details.

[tblah]

The first restriction on the allocate clause listed here is

For any list item that is specified in the allocate clause on a directive other than the allocators directive, a data-sharing attribute clause that may create a private copy of that list item must be specified on the same directive.

I think the data sharing attribute clause referred to here is task_reduction (which makes sense with allocate because it will make copies). I don't think we can apply it to data-sharing attribute clauses on a different directive.

Did your patch originate from a real application containing code accepted by other compilers?