[Flang] [OpenMP] [Semantics] Change SIMD ALIGNED clause support from parsing a std::list<Name> to OmpObjectlist

[raghavendhra]

Summary

This is an assisting patch which is implemented to address review comment to switch std::list<Name> to OmpObjectlist from https://reviews.llvm.org/D142722.

Also addressed a semantic check https://github.com/llvm/llvm-project/issues/61161 OpenMP 5.2 standard states that only pointer variables (C_PTR, Cray pointers, POINTER or ALLOCATABLE items) can appear in SIMD aligned clause (section 5.11). And not to allow common block names on an ALIGNED clause.

[raghavendhra]

Rebasing

[raghavendhra]

Rebase

[raghavendhra]

Rebase

[NimishMishra]

Thank you for the patch. Should we add tests for this?

[raghavendhra]

In D152637#4426952, @NimishMishra wrote:

Thank you for the patch. Should we add tests for this?

There is a test already for SIMD ALIGNED and I added few lines to the file simd_aligned.F90 in my latest commit to test the new restriction I implemented regarding common block names not being referred within an ALIGNED clause.

[raghavendhra]

Rebase and addressed review comment to add a test case for not allowing common block name in an ALIGNED clause.

[domada]

Hi @raghavendhra
could you also address this issue: https://github.com/llvm/llvm-project/issues/61161 ?

[clementval]

Please update the title. std::list<Name> is not a namelist in the Fortran sense.

[raghavendhra]

In D152637#4445295, @clementval wrote:

Please update the title. std::list<Name> is not a namelist in the Fortran sense.

Done!

[raghavendhra]

In D152637#4434384, @domada wrote:

Hi @raghavendhra
could you also address this issue: https://github.com/llvm/llvm-project/issues/61161 ?

Addressed this in latest revision. Thanks for bringing that into attention.

[raghavendhra]

Addressed https://github.com/llvm/llvm-project/issues/61161 and Rebased the patch

[domada]

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

200

nit: unused variable ‘commonBlock’

[raghavendhra]

Thanks for catching that Dominik. :)

[raghavendhra]

Removed unused variable and rebased the patch.

[kiranchandramohan]

Please change the summary of the patch to include everything that is there in this patch.
-> Switch to OmpObjectList and why
-> Semantic check

LG.

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

198–199

This check can either be hoisted into the if or could be a wrapping condition over the rest of the else if.

if (const auto *name{parser::Unwrap<parser::Name>(ompObject)} && name->symbol) {
  if (FindCommonBlockContaining(*(name->symbol))) {
  } else if ...
  ...
}

if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
  if (name->symbol) {
    if (FindCommonBlockContaining(*(name->symbol))) {
    } ...
    ...
  }
}

[raghavendhra]

Final rebase before commit.

flang/include/flang/Parser/parse-tree.h

ENUM_CLASS(DirectiveNameModifier, Parallel, Target, TargetEnterData,
       TargetExitData, TargetData, TargetUpdate, Taskloop, Task)
   std::tuple<std::optional<DirectiveNameModifier>, ScalarLogicalExpr> t;
 };
 
 // 2.8.1 aligned-clause -> ALIGNED (variable-name-list[ : scalar-constant])
 struct OmpAlignedClause {
   TUPLE_CLASS_BOILERPLATE(OmpAlignedClause);
   CharBlock source;
-  std::tuple<std::list<Name>, std::optional<ScalarIntConstantExpr>> t;
+  std::tuple<OmpObjectList, std::optional<ScalarIntConstantExpr>> t;
 };
 
 // 2.9.5 order-clause -> ORDER ([order-modifier :]concurrent)
 struct OmpOrderModifier {
   UNION_CLASS_BOILERPLATE(OmpOrderModifier);
   ENUM_CLASS(Kind, Reproducible, Unconstrained)
   std::variant<Kind> u;
 };

flang/lib/Parser/openmp-parsers.cpp

construct<OmpLinearClause>(
         construct<OmpLinearClause>(construct<OmpLinearClause::WithModifier>(
             Parser<OmpLinearModifier>{}, parenthesized(nonemptyList(name)),
             maybe(":" >> scalarIntConstantExpr))) ||
         construct<OmpLinearClause>(construct<OmpLinearClause::WithoutModifier>(
             nonemptyList(name), maybe(":" >> scalarIntConstantExpr)))))
 
 // 2.8.1 ALIGNED (list: alignment)
 TYPE_PARSER(construct<OmpAlignedClause>(
-    nonemptyList(name), maybe(":" >> scalarIntConstantExpr)))
+    Parser<OmpObjectList>{}, maybe(":" >> scalarIntConstantExpr)))
 
 // 2.9.5 ORDER ([order-modifier :]concurrent)
 TYPE_PARSER(construct<OmpOrderModifier>(
     "REPRODUCIBLE" >> pure(OmpOrderModifier::Kind::Reproducible)) ||
     construct<OmpOrderModifier>(
     "UNCONSTRAINED" >> pure(OmpOrderModifier::Kind::Unconstrained)))
 
 TYPE_PARSER(construct<OmpOrderClause>(

flang/lib/Parser/unparse.cpp

void Unparse(const OmpScheduleClause &x) {
     Walk(std::get<OmpScheduleClause::ScheduleType>(x.t));
     Walk(",", std::get<std::optional<ScalarIntExpr>>(x.t));
   }
   void Unparse(const OmpDeviceClause &x) {
     Walk(std::get<std::optional<OmpDeviceClause::DeviceModifier>>(x.t), ":");
     Walk(std::get<ScalarIntExpr>(x.t));
   }
   void Unparse(const OmpAlignedClause &x) {
-    Walk(std::get<std::list<Name>>(x.t), ",");
+    Walk(std::get<OmpObjectList>(x.t));
+    Put(",");
     Walk(std::get<std::optional<ScalarIntConstantExpr>>(x.t));
   }
   void Unparse(const OmpIfClause &x) {
     Walk(std::get<std::optional<OmpIfClause::DirectiveNameModifier>>(x.t), ":");
     Walk(std::get<ScalarLogicalExpr>(x.t));
   }
   void Unparse(const OmpLinearClause::WithoutModifier &x) {
     Walk(x.names, ", ");

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

auto checkMultipleOcurrence = [&](const std::list<parser::Name> &nameList,
     }
   };
 
   // Aligned clause
   auto alignedClauses{FindClauses(llvm::omp::Clause::OMPC_aligned)};
   for (auto itr = alignedClauses.first; itr != alignedClauses.second; ++itr) {
     const auto &alignedClause{
         std::get<parser::OmpClause::Aligned>(itr->second->u)};
-    const auto &alignedNameList{
-        std::get<std::list<parser::Name>>(alignedClause.v.t)};
+    const auto &alignedList{std::get<0>(alignedClause.v.t)};
+    std::list<parser::Name> alignedNameList;
+    for (const auto &ompObject : alignedList.v) {
+      if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+        if (name->symbol) {
+          if (FindCommonBlockContaining(*(name->symbol))) {
+            context_.Say(itr->second->source,
+                "'%s' is a common block name and can not appear in an "
+                "ALIGNED clause"_err_en_US,
+                name->ToString());
+          } else if (!(IsBuiltinCPtr(*(name->symbol)) ||
+                         IsAllocatableOrPointer(
+                             (name->symbol->GetUltimate())))) {
+            context_.Say(itr->second->source,
+                "'%s' in ALIGNED clause must be of type C_PTR, POINTER or "
+                "ALLOCATABLE"_err_en_US,
+                name->ToString());
+          } else {
+            alignedNameList.push_back(*name);
+          }
+        } else {
+          // The symbol is null, return early
+          return;
+        }
+      }
+    }
     checkMultipleOcurrence(alignedNameList, itr->second->source, "ALIGNED");
   }
 
   // Nontemporal clause
   auto nonTemporalClauses{FindClauses(llvm::omp::Clause::OMPC_nontemporal)};
   for (auto itr = nonTemporalClauses.first; itr != nonTemporalClauses.second;
        ++itr) {
     const auto &nontempClause{
const parser::OmpObjectList *OmpStructureChecker::GetOmpObjectList(
   using MemberObjectListClauses = std::tuple<parser::OmpClause::Copyprivate,
       parser::OmpClause::Copyin, parser::OmpClause::Firstprivate,
       parser::OmpClause::From, parser::OmpClause::Lastprivate,
       parser::OmpClause::Link, parser::OmpClause::Private,
       parser::OmpClause::Shared, parser::OmpClause::To,
       parser::OmpClause::UseDevicePtr, parser::OmpClause::UseDeviceAddr>;
 
   // Clauses with OmpObjectList in the tuple
-  using TupleObjectListClauses = std::tuple<parser::OmpClause::Allocate,
-      parser::OmpClause::Map, parser::OmpClause::Reduction>;
+  using TupleObjectListClauses =
+      std::tuple<parser::OmpClause::Allocate, parser::OmpClause::Map,
+          parser::OmpClause::Reduction, parser::OmpClause::Aligned>;
 
   // TODO:: Generate the tuples using TableGen.
   // Handle other constructs with OmpObjectList such as OpenMPThreadprivate.
   return common::visit(
       common::visitors{
           [&](const auto &x) -> const parser::OmpObjectList * {
             using Ty = std::decay_t<decltype(x)>;
             if constexpr (common::HasMember<Ty, MemberObjectListClauses>) {

flang/lib/Semantics/resolve-directives.cpp

if (const auto *procD{parser::Unwrap<parser::ProcedureDesignator>(opr.u)}) {
       }
     }
     const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
     ResolveOmpObjectList(objList, Symbol::Flag::OmpReduction);
     return false;
   }
 
   bool Pre(const parser::OmpAlignedClause &x) {
-    const auto &alignedNameList{std::get<std::list<parser::Name>>(x.t)};
-    ResolveOmpNameList(alignedNameList, Symbol::Flag::OmpAligned);
+    const auto &alignedNameList{std::get<parser::OmpObjectList>(x.t)};
+    ResolveOmpObjectList(alignedNameList, Symbol::Flag::OmpAligned);
     return false;
   }
 
   bool Pre(const parser::OmpClause::Nontemporal &x) {
     const auto &nontemporalNameList{x.v};
     ResolveOmpNameList(nontemporalNameList, Symbol::Flag::OmpNontemporal);
     return false;
   }

flang/test/Semantics/OpenMP/clause-validity01.f90

 ! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
 use omp_lib
 ! Check OpenMP clause validity for the following directives:
 !
 !    2.5 PARALLEL construct
 !    2.7.1 Loop construct
 !    ...
 
+  use iso_c_binding
   integer :: b = 128
+  integer, allocatable :: allc
+  type(C_PTR) :: cpt
   integer :: z, c = 32
   integer, parameter :: num = 16
   real(8) :: arrayA(256), arrayB(512)
 
   integer(omp_memspace_handle_kind) :: xy_memspace = omp_default_mem_space
   type(omp_alloctrait) :: xy_traits(1) = [omp_alloctrait(omp_atk_alignment,64)]
   integer(omp_allocator_handle_kind) :: xy_alloc
   xy_alloc = omp_init_allocator(xy_memspace, 1, xy_traits)
!                      collapse-clause
 
   !ERROR: The parameter of the SIMDLEN clause must be a constant positive integer expression
   !$omp simd simdlen(-1)
   do i = 1, N
      a = 3.14
   enddo
 
   !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
-  !$omp simd aligned(b:-2)
+  !$omp simd aligned(cpt:-2)
   do i = 1, N
      a = 3.14
   enddo
 
   !$omp parallel
   !ERROR: The parameter of the SIMDLEN clause must be less than or equal to the parameter of the SAFELEN clause
   !$omp simd safelen(1+1) simdlen(1+2)
   do i = 1, N
!                               simd-clause
   !$omp end taskloop
 
   !ERROR: GRAINSIZE and NUM_TASKS clauses are mutually exclusive and may not appear on the same TASKLOOP SIMD directive
   !$omp taskloop simd num_tasks(3) grainsize(2)
   do i = 1,N
      a = 3.14
   enddo
 
+  allocate(allc)
   !ERROR: The parameter of the SIMDLEN clause must be a constant positive integer expression
   !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
-  !$omp taskloop simd simdlen(-1) aligned(a:-2)
+  !$omp taskloop simd simdlen(-1) aligned(allc:-2)
   do i = 1, N
-     a = 3.14
+     allc = 3.14
   enddo
 
   !$omp target enter data map(alloc:A) device(0) 
   !$omp target exit data map(delete:A) device(0) 
 
 end program

flang/test/Semantics/OpenMP/simd-aligned.f90

 ! RUN: %python %S/../test_errors.py %s %flang -fopenmp
 
 ! OpenMP Version 4.5
 ! 2.8.1 simd Construct
 ! Semantic error for correct test case
 
 program omp_simd
-  integer i, j, k
+  integer i, j, k, c, d(100)
   integer, allocatable :: a(:), b(:)
+  common /cmn/ c
 
   allocate(a(10))
   allocate(b(10))
 
   !ERROR: List item 'a' present at multiple ALIGNED clauses
   !$omp simd aligned(a, a)
   do i = 1, 10
     a(i) = i
program omp_simd
   do i = 1, 10
     a(i) = i
     b(i) = i
   end do
   !$omp end simd
 
   print *, a
 
+  !ERROR: 'c' is a common block name and can not appear in an ALIGNED clause
+  !$omp simd aligned(c)
+  do i = 1, 10
+    c = 5
+  end do
+  !$omp end simd
+
+  !ERROR: 'd' in ALIGNED clause must be of type C_PTR, POINTER or ALLOCATABLE
+  !$omp simd aligned(d:100)
+  do i = 1, 100
+    d(i) = i
+  end do
+
 end program omp_simd