[Flang][OpenMP 4.5] Add semantic check for OpenMP shared and private clause

[yhegde]

Summary

Semantic check for OpenMP 4.5 - 2.15.3.2 and 2.15.3.3 shared and private clause

Test cases : omp-parallel-shared.f90

        omp-parallel-shared02.f90
        omp-parallel-shared03.f90
        omp-parallel-shared04.f90    
and  omp-parallel-private.f90
        omp-parallel-private02.f90
        omp-parallel-private03.f90
        omp-parallel-private04.f90

[clementval]

flang/lib/Semantics/check-directive-structure.h

61 ↗

(On Diff #298153)

Since this is OpenMP specific, it should be in check-omp-structure and not here.

66 ↗

(On Diff #298153)

Same here. Should be moved to check-omp-structure.h

[clementval]

flang/lib/Semantics/check-directive-structure.h

199 ↗

(On Diff #298153)

OmpObjectList is specific to OpenMP and doesn't fit in check-directive-structure.h

238 ↗

(On Diff #298153)

In the rest of Flang there is no blank line between each specific visitor so you might want to stick with the same coding style.

[tskeith]

flang/lib/Semantics/check-directive-structure.h

212 ↗

(On Diff #298153)

This code just seems to be trying to determine if ompObject is a StructureComponent. If so, it could be simplified to:

if (parser::Unwrap<parser::StructureComponent>(ompObject)) {

[kiranchandramohan]

Thanks for this patch. Did you miss the array element check?

flang/lib/Semantics/check-omp-structure.cpp

413

Nit: you can skip the standard section number here.

421

Nit: you can skip the standard section number here.

flang/lib/Semantics/check-omp-structure.h

165

Nit: You can skip the section number here.

flang/test/Semantics/omp-parallel-private.f90

9

Nit: alignment

15–18

Is this loop required for this test?

22–25

Nit: Alignment.

flang/test/Semantics/omp-parallel-shared.f90

9

Nit: alignment

12–16

Is this loop required for this test?

[yhegde]

1. Array element check added along with test cases to test the same.

[yhegde]

clang format

[Prashanth]

This comment was removed by Prashanth.

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

435

Clause names are usually uppercase in the error message.

[kiranchandramohan]

LGTM.
Please wait for approval from @clementval after uppercasing the clause names.

flang/lib/Semantics/check-omp-structure.cpp

422

Nit: Would a better name be CheckIsVarPartOfAnotherVar?

flang/lib/Semantics/check-omp-structure.h

165

Nit: Can this be private?

[yhegde]

1. Function name changed to CheckIsVarPartOfAnotherVar
2. Clause names in uppercase.

[clementval]

Ok from my side. Maybe @kiranchandramohan wants to double check the latest changes.

[kiranchandramohan]

LGTM.

[kiranchandramohan]

@yhegde please submit. If you require any inputs then please let me know. This will be helpful for D91159.

[yhegde]

Thanks a lot for the review comments.
Requesting a quick review of this accepted patch. I had to revert the code part related to Shared and Private clause from the review https://reviews.llvm.org/D90324.

Test cases : omp-parallel-shared.01f90

        omp-parallel-shared02.f90
        omp-parallel-shared03.f90
        omp-parallel-shared04.f90    
and  omp-parallel-private.01f90
        omp-parallel-private02.f90
        omp-parallel-private03.f90
        omp-parallel-private04.f90

[yhegde]

Shall I assume there are no issues with this patch. ?

[kiranchandramohan]

No issues, please go ahead.

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   void Enter(const parser::OmpDistScheduleClause &);
   void Enter(const parser::OmpIfClause &);
   void Enter(const parser::OmpLinearClause &);
   void Enter(const parser::OmpMapClause &);
   void Enter(const parser::OmpProcBindClause &);
   void Enter(const parser::OmpReductionClause &);
   void Enter(const parser::OmpScheduleClause &);
 
 private:
 
+  void CheckIsVarPartOfAnotherVar(const parser::OmpObjectList &objList);
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
 
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,
       const parser::OmpScheduleModifierType::ModType &);
 
   llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
 };
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_

flang/lib/Semantics/check-omp-structure.cpp

if (llvm::omp::doSimdSet.test(GetContext().directive)) {
           ContextDirectiveAsFortran());
     }
   }
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::Priority &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_priority);
   RequiresPositiveParameter(llvm::omp::Clause::OMPC_priority, x.v);
 }
-void OmpStructureChecker::Enter(const parser::OmpClause::Private &) {
+void OmpStructureChecker::Enter(const parser::OmpClause::Private &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_private);
+  CheckIsVarPartOfAnotherVar(x.v);
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::Safelen &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_safelen);
   RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_safelen, x.v);
 }
-void OmpStructureChecker::Enter(const parser::OmpClause::Shared &) {
+void OmpStructureChecker::Enter(const parser::OmpClause::Shared &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_shared);
+  CheckIsVarPartOfAnotherVar(x.v);
+}
+void OmpStructureChecker::CheckIsVarPartOfAnotherVar(
+    const parser::OmpObjectList &objList) {
+
+  for (const auto &ompObject : objList.v) {
+    std::visit(
+        common::visitors{
+            [&](const parser::Designator &designator) {
+              if (std::get_if<parser::DataRef>(&designator.u)) {
+                if ((parser::Unwrap<parser::StructureComponent>(ompObject)) ||
+                    (parser::Unwrap<parser::ArrayElement>(ompObject))) {
+                  context_.Say(GetContext().clauseSource,
+                      "A variable that is part of another variable (as an "
+                      "array or structure element)"
+                      " cannot appear in a PRIVATE or SHARED clause."_err_en_US);
+                }
+              }
+            },
+            [&](const parser::Name &name) {},
+        },
+        ompObject.u);
+  }
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::Simdlen &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_simdlen);
   RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_simdlen, x.v);
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::ThreadLimit &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_thread_limit);
   RequiresPositiveParameter(llvm::omp::Clause::OMPC_thread_limit, x.v);

flang/test/Semantics/omp-parallel-private.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.3 parallel private Clause
+program omp_parallel_private
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+    integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel private(my_var%array)
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    my_var%array(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_private

flang/test/Semantics/omp-parallel-private02.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.3 parallel private Clause
+program omp_parallel_private
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  integer :: array(10)
+
+  do i = 1, 10
+    array(i) = i
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel private(array(i))
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    array(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_private

flang/test/Semantics/omp-parallel-private03.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.3 parallel private Clause
+program omp_parallel_private
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+    integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  real :: arr(10)
+  integer :: intx = 10
+
+  do i = 1, 10
+    arr(i) = 0.0
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel private(arr(i),intx)
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    my_var%array(i) = k+intx
+    arr(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_private

flang/test/Semantics/omp-parallel-private04.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.3 parallel private Clause
+program omp_parallel_private
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+    integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  real :: arr(10)
+  integer :: intx = 10
+
+  do i = 1, 10
+    arr(i) = 0.0
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel private(arr,intx,my_var%array(1))
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    my_var%array(i) = k+intx
+    arr(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_private

flang/test/Semantics/omp-parallel-shared.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.2 parallel shared Clause
+program omp_parallel_shared
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+  integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel shared(my_var%array)
+  do i = 1, 10
+    c(i) = a(i) + b(i) +  k
+    my_var%array(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_shared

flang/test/Semantics/omp-parallel-shared02.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.2 parallel shared Clause
+program omp_parallel_shared
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  integer :: array(10)
+
+  do i = 1, 10
+    array(i) = i
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel shared(array(i))
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    array(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_shared

flang/test/Semantics/omp-parallel-shared03.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.2 parallel shared Clause
+program omp_parallel_shared
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+    integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  real :: arr(10)
+  integer :: intx = 10
+
+  do i = 1, 10
+    arr(i) = 0.0
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel shared(arr(i),intx)
+  do i = 1, 10
+     c(i) = a(i) + b(i) + k
+     my_var%array(i) = k+intx
+     arr(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_shared

flang/test/Semantics/omp-parallel-shared04.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.2 parallel shared Clause
+program omp_parallel_shared
+  integer :: i, j, a(10), b(10), c(10)
+  integer :: k = 10
+  type my_type
+    integer :: array(10)
+  end type my_type
+
+  type(my_type) :: my_var
+
+  real :: arr(10)
+  integer :: intx = 10
+
+  do i = 1, 10
+    arr(i) = 0.0
+  end do
+
+  !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a PRIVATE or SHARED clause.
+  !$omp parallel shared(arr,intx,my_var%array(1))
+  do i = 1, 10
+    c(i) = a(i) + b(i) + k
+    my_var%array(i) = k+intx
+    arr(i) = k
+  end do
+  !$omp end parallel
+end program omp_parallel_shared