[flang][openacc] Add missing loop construct restriction and validity tests

[clementval]

Summary

Add restriction on loop construct associated with DO CONCURRENT. Add couple of tests to ensure
clause validity checks.

[sameeranjoshi]

The tests look good, just a few comments/queries?
I am really not familiar with canonicalization, but I could grasp what's defined in this patch.

flang/lib/Semantics/canonicalize-acc.cpp

68

If that's a restriction why is this not in the form of a semantic error message?

87

Which standard ?
There are patches for 3.1 up gradation, will line numbers change in that case?
Is 3.0 assumed when no reference?

99

A C++ beginners query -
Why use std::get_if over std::holds_alternative if we are not utilizing the return pointer?

102

nit:Was that or instead of and ?

[clementval]

flang/lib/Semantics/canonicalize-acc.cpp

68

The error message is issued in CheckDoConcurrentClauseRestriction in case there is a tile clause associated. Here we just bypass the logic and we do not issue the message a second time.

87

From no one all reference are from 3.1. Right now it's like a transition phase until patch D92120 is approved and landed.

99

std::holds_alternative would be fine in this case. I'll update the patch. Thx.

102

I just use the text from the standard. or would work as well.

[clementval]

flang/lib/Semantics/canonicalize-acc.cpp

87

From now on ... and not "no one"!

[clementval]

Address review comments + rebase

[sameeranjoshi]

Thanks for clarifications.
Approving it just take care of the build issue before merging.
LGTM.

flang/lib/Semantics/canonicalize-acc.cpp

99

Please check syntax this doesn't build.
no &.

[clementval]

Rebase + fix build problem

[clementval]

flang/lib/Semantics/canonicalize-acc.cpp

99

Thanks for catching this. Don't know what I did with this :-)

flang/lib/Semantics/canonicalize-acc.cpp

void CheckTileClauseRestriction(const C &x) {
     for (const auto &clause : accClauseList.v) {
       if (const auto *tileClause =
               std::get_if<parser::AccClause::Tile>(&clause.u)) {
         const parser::AccTileExprList &tileExprList = tileClause->v;
         const std::list<parser::AccTileExpr> &listTileExpr = tileExprList.v;
         std::size_t tileArgNb = listTileExpr.size();
 
         const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
+        if (outer->IsDoConcurrent())
+          return; // Tile is not allowed on DO CONURRENT
         for (const parser::DoConstruct *loop{&*outer}; loop && tileArgNb > 0;
              --tileArgNb) {
           const auto &block{std::get<parser::Block>(loop->t)};
           const auto it{block.begin()};
           loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
                                    : nullptr;
         }
 
         if (tileArgNb > 0) {
           messages_.Say(beginLoopDirective.source,
               "The loop construct with the TILE clause must be followed by %d "
               "tightly-nested loops"_err_en_US,
               listTileExpr.size());
         }
       }
     }
   }
 
+  // Check constraint on line 1835 in Section 2.9
+  // A tile and collapse clause may not appear on loop that is associated with
+  // do concurrent.
+  template <typename C, typename D>
+  void CheckDoConcurrentClauseRestriction(const C &x) {
+    const auto &doCons{std::get<std::optional<parser::DoConstruct>>(x.t)};
+    if (!doCons->IsDoConcurrent())
+      return;
+    const auto &beginLoopDirective = std::get<D>(x.t);
+    const auto &accClauseList =
+        std::get<parser::AccClauseList>(beginLoopDirective.t);
+    for (const auto &clause : accClauseList.v) {
+      if (std::get_if<parser::AccClause::Collapse>(&clause.u) ||
+          std::get_if<parser::AccClause::Tile>(&clause.u)) {
+        messages_.Say(beginLoopDirective.source,
+            "TILE and COLLAPSE clause may not appear on loop construct "
+            "associated with DO CONCURRENT"_err_en_US);
+      }
+    }
+  }
+
   void RewriteOpenACCLoopConstruct(parser::OpenACCLoopConstruct &x,
       parser::Block &block, parser::Block::iterator it) {
     // Check the sequence of DoConstruct in the same iteration
     //
     // Original:
     //   ExecutableConstruct -> OpenACCConstruct -> OpenACCLoopConstruct
     //     ACCBeginLoopDirective
     //   ExecutableConstruct -> DoConstruct
if (++nextIt != block.end()) {
           std::get<std::optional<parser::DoConstruct>>(x.t) =
               std::move(*doCons);
           nextIt = block.erase(nextIt);
         } else {
           messages_.Say(dir.source,
               "DO loop after the %s directive must have loop control"_err_en_US,
               parser::ToUpperCaseLetters(dir.source.ToString()));
         }
+
+        CheckDoConcurrentClauseRestriction<parser::OpenACCLoopConstruct,
+            parser::AccBeginLoopDirective>(x);
         CheckTileClauseRestriction<parser::OpenACCLoopConstruct,
             parser::AccBeginLoopDirective>(x);
+
         return; // found do-loop
       }
     }
     messages_.Say(dir.source,
         "A DO loop must follow the %s directive"_err_en_US,
         parser::ToUpperCaseLetters(dir.source.ToString()));
   }
 
if (++nextIt != block.end()) {
               block.erase(nextIt);
             }
           }
         } else {
           messages_.Say(dir.source,
               "DO loop after the %s directive must have loop control"_err_en_US,
               parser::ToUpperCaseLetters(dir.source.ToString()));
         }
+
+        CheckDoConcurrentClauseRestriction<parser::OpenACCCombinedConstruct,
+            parser::AccBeginCombinedDirective>(x);
         CheckTileClauseRestriction<parser::OpenACCCombinedConstruct,
             parser::AccBeginCombinedDirective>(x);
+
         return; // found do-loop
       }
     }
     messages_.Say(dir.source,
         "A DO loop must follow the %s directive"_err_en_US,
         parser::ToUpperCaseLetters(dir.source.ToString()));
   }
 

flang/test/Semantics/acc-canonicalization-validity.f90

 ! RUN: %S/test_errors.sh %s %t %f18 -fopenacc
 
 ! Check OpenACC canonalization validity for the construct defined below:
 !   2.9 Loop
 !   2.11 Parallel Loop
 !   2.11 Kernels Loop
 !   2.11 Serial Loop
 
 program openacc_clause_validity
 
   implicit none
 
   integer :: i, j
   integer :: N = 256
   real(8) :: a(256)
+  real(8) :: aa(256, 256)
 
   !ERROR: A DO loop must follow the LOOP directive
   !$acc loop
   i = 1
 
   !ERROR: DO loop after the LOOP directive must have loop control
   !$acc loop
   do
program openacc_clause_validity
   !$acc end parallel
 
   !ERROR: The loop construct with the TILE clause must be followed by 2 tightly-nested loops
   !$acc parallel loop tile(2, 2)
   do i = 1, N
     a(i) = 3.14
   end do
 
+  !$acc parallel
+  !ERROR: TILE and COLLAPSE clause may not appear on loop construct associated with DO CONCURRENT
+  !$acc loop collapse(2)
+  do concurrent (i = 1:N, j = 1:N)
+    aa(i, j) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
+  !ERROR: TILE and COLLAPSE clause may not appear on loop construct associated with DO CONCURRENT
+  !$acc loop tile(2, 2)
+  do concurrent (i = 1:N, j = 1:N)
+    aa(i, j) = 3.14
+  end do
+  !$acc end parallel
+
 end program openacc_clause_validity

flang/test/Semantics/acc-clause-validity.f90

program openacc_clause_validity
   !ERROR: INDEPENDENT and SEQ clauses are mutually exclusive and may not appear on the same LOOP directive
   !$acc loop seq independent
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
+  !ERROR: SEQ and AUTO clauses are mutually exclusive and may not appear on the same LOOP directive
+  !$acc loop auto seq
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
   !$acc loop tile(2)
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel loop tile(2)
   do i = 1, N
program openacc_clause_validity
   !$acc parallel
   !$acc loop auto
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
+  !ERROR: At most one VECTOR clause can appear on the LOOP directive
+  !$acc loop vector vector(128)
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
   !$acc loop vector
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
   !$acc loop vector(10)
program openacc_clause_validity
   !$acc parallel
   !$acc loop vector(length: vector_size)
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
+  !ERROR: At most one WORKER clause can appear on the LOOP directive
+  !$acc loop worker worker(10)
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
   !$acc loop worker
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
   !$acc loop worker(10)
program openacc_clause_validity
   !$acc parallel
   !$acc loop worker(num: worker_size)
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
+  !ERROR: At most one GANG clause can appear on the LOOP directive
+  !$acc loop gang gang(gang_size)
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
   !$acc loop gang(gang_size)
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
   !$acc parallel
   !$acc loop gang(num: gang_size)
program openacc_clause_validity
   !$acc parallel
   !ERROR: Clause GANG is not allowed if clause SEQ appears on the LOOP directive
   !$acc loop gang seq
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
+  !$acc parallel
+  !ERROR: Clause WORKER is not allowed if clause SEQ appears on the LOOP directive
+  !$acc loop worker seq
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
+  !$acc parallel
+  !ERROR: Clause VECTOR is not allowed if clause SEQ appears on the LOOP directive
+  !$acc loop vector seq
+  do i = 1, N
+    a(i) = 3.14
+  end do
+  !$acc end parallel
+
   !ERROR: Clause IF is not allowed after clause DEVICE_TYPE on the PARALLEL directive
   !$acc parallel device_type(*) if(.TRUE.)
   !$acc loop
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel