[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.

413

Nit: you can skip the standard section number here.

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

173

Nit: You can skip the section number here.

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

8 ↗

(On Diff #298335)

Nit: alignment

14–17 ↗

(On Diff #298335)

Is this loop required for this test?

21–24 ↗

(On Diff #298335)

Nit: Alignment.

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

8 ↗

(On Diff #298335)

Nit: alignment

11–15 ↗

(On Diff #298335)

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

427

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

414

Nit: Would a better name be CheckIsVarPartOfAnotherVar?

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

173

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:
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
 
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,
       const parser::OmpScheduleModifierType::ModType &);
   void CheckAllowedMapTypes(const parser::OmpMapType::Type &,
       const std::list<parser::OmpMapType::Type> &);
   llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
 
   void CheckDependList(const parser::DataRef &);
   void CheckDependArraySection(
       const common::Indirection<parser::ArrayElement> &, const parser::Name &);
+
+  void CheckIsVarPartOfAnotherVar(const parser::OmpObjectList &objList);
 };
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_

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

 CHECK_SIMPLE_CLAUSE(From, OMPC_from)
 CHECK_SIMPLE_CLAUSE(Inbranch, OMPC_inbranch)
 CHECK_SIMPLE_CLAUSE(IsDevicePtr, OMPC_is_device_ptr)
 CHECK_SIMPLE_CLAUSE(Lastprivate, OMPC_lastprivate)
 CHECK_SIMPLE_CLAUSE(Link, OMPC_link)
 CHECK_SIMPLE_CLAUSE(Mergeable, OMPC_mergeable)
 CHECK_SIMPLE_CLAUSE(Nogroup, OMPC_nogroup)
 CHECK_SIMPLE_CLAUSE(Notinbranch, OMPC_notinbranch)
-CHECK_SIMPLE_CLAUSE(Private, OMPC_private)
-CHECK_SIMPLE_CLAUSE(Shared, OMPC_shared)
 CHECK_SIMPLE_CLAUSE(To, OMPC_to)
 CHECK_SIMPLE_CLAUSE(Uniform, OMPC_uniform)
 CHECK_SIMPLE_CLAUSE(Untied, OMPC_untied)
 CHECK_SIMPLE_CLAUSE(UseDevicePtr, OMPC_use_device_ptr)
 CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
 CHECK_SIMPLE_CLAUSE(Acquire, OMPC_acquire)
 CHECK_SIMPLE_CLAUSE(SeqCst, OMPC_seq_cst)
 CHECK_SIMPLE_CLAUSE(Release, OMPC_release)
if (llvm::omp::doSimdSet.test(GetContext().directive)) {
       context_.Say(GetContext().clauseSource,
           "No ORDERED clause with a parameter can be specified "
           "on the %s directive"_err_en_US,
           ContextDirectiveAsFortran());
     }
   }
 }
 
+void OmpStructureChecker::Enter(const parser::OmpClause::Shared &x) {
+  CheckAllowed(llvm::omp::Clause::OMPC_shared);
+  CheckIsVarPartOfAnotherVar(x.v);
+}
+void OmpStructureChecker::Enter(const parser::OmpClause::Private &x) {
+  CheckAllowed(llvm::omp::Clause::OMPC_private);
+  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);
+  }
+}
 // Following clauses have a seperate node in parse-tree.h.
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAllocateClause, OMPC_allocate)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDefaultClause, OMPC_default)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDistScheduleClause, OMPC_dist_schedule)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpNowait, OMPC_nowait)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpProcBindClause, OMPC_proc_bind)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpReductionClause, OMPC_reduction)
 

flang/test/Semantics/omp-parallel-private01.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-shared01.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