[PATCH] D91920: [Flang] [OpenMP] Add semantic checks for OpenMP firstprivate , lastprivate and copyprivate clauses

[Kiran Chandramohan via Phabricator via llvm-commits]

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

I believe the checks have to be made more precise. Please have a look at the comments.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:857
+
+void OmpStructureChecker::CheckPrivateSymbolsInOuterCxt(
+    SymbolSourceMap &currSymbols, const OmpDirectiveSet &currDirSet,
----------------
Can you check why we cannot catch the error in the following test. gfortran seems able to.
```
program mn
integer :: a(10), b(10), c(10)
!$omp parallel private(a,b)
call tmp()
!$omp end parallel
contains
  subroutine tmp()
  !$omp do firstprivate(b)
  do i = 1, 10
    c(i) = a(i) + b(i) + i
  end do
  !$omp end do
  end subroutine
end program
```


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:864
+  if (currDirSet.test(GetContext().directive)) {
+    if (auto *enclosingContext{GetEnclosingDirContext()}) {
+      if (enclosingDirSet.test(enclosingContext->directive)) {
----------------
Is there an assumption here that the enclosing directive is the parallel/teams region?

Can it happen that the enclosing immediate directive is not the parallel/teams region to which the worksharing construct binds? Something like the following? 
```
parallel private(x)
  some_omp_construct
    worksharing_construct firstprivate(x)
```


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:865
+    if (auto *enclosingContext{GetEnclosingDirContext()}) {
+      if (enclosingDirSet.test(enclosingContext->directive)) {
+        for (auto it{enclosingContext->clauseInfo.begin()};
----------------
I think whether this check should be performed is dependent on the triple enclosing_dir, enclosing_clause, cur_dir.

So there should be no error for the following code.

```
program omp_firstprivate

  integer :: i, a(10), b(10), c(10)

  a = 10
  b = 20

  !$omp parallel private(a, b)
  !$omp task firstprivate(a)
  do i = 1, 10
    a(i) = a(i) + b(i) - i
  end do
  !$omp end task
  !$omp end parallel
end program
```

But there should be an error for the following code.
```
program omp_firstprivate
  integer :: i, a
  a = 10
  i = 2
  !$omp parallel reduction(+:a)
  !$omp task firstprivate(a)
    a = a + i
  !$omp end task
  !$omp end parallel
end program
```


================
Comment at: flang/test/Semantics/omp-copyprivate03.f90:27
+
+  !$omp parallel sections private(k)
+  !$omp section
----------------
A threadprivate variable cannot appear in a private clause.


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

https://reviews.llvm.org/D91920