[flang][openacc] Accept !$acc end loop

Some compilers accept `!$acc end loop` associated with an `!$acc loop`
directive. This patch updates the acc loop parser to accept it as well.
The parser is also updated to be stricter on the following statement
to match the OpenACC combined construct parser.
The rewrite canonicalization is not a rewrite anymore and the naming
will be updated in a follow up patch for the Loop and Combined constructs.

Reviewed By: razvanlupusoru

Differential Revision: https://reviews.llvm.org/D159015

Author: clementval

flang/docs/OpenACC.md

@@ -17,3 +17,4 @@
 * The end directive for combined construct can omit the `loop` keyword.
 * An `!$acc routine` with no parallelism clause is treated as if the `seq`
   clause was present.
+* `!$acc end loop` does not trigger a parsing error and is just ignored.

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

@@ -105,6 +105,7 @@ class ParseTreeDumper {
   NODE(parser, AccTileExpr)
   NODE(parser, AccTileExprList)
   NODE(parser, AccLoopDirective)
+  NODE(parser, AccEndLoop)
   NODE(parser, AccWaitArgument)
   static std::string GetNodeName(const llvm::acc::Directive &x) {
     return llvm::Twine(

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

@@ -4244,11 +4244,10 @@ struct OpenACCDeclarativeConstruct {
 };
 
 // OpenACC directives enclosing do loop
+EMPTY_CLASS(AccEndLoop);
 struct OpenACCLoopConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenACCLoopConstruct);
-  OpenACCLoopConstruct(AccBeginLoopDirective &&a)
-      : t({std::move(a), std::nullopt}) {}
-  std::tuple<AccBeginLoopDirective, std::optional<DoConstruct>> t;
+  std::tuple<AccBeginLoopDirective, DoConstruct, std::optional<AccEndLoop>> t;
 };
 
 struct OpenACCStandaloneConstruct {

flang/lib/Parser/openacc-parsers.cpp

@@ -150,8 +150,13 @@ TYPE_PARSER(sourced(construct<AccLoopDirective>(
 TYPE_PARSER(construct<AccBeginLoopDirective>(
     sourced(Parser<AccLoopDirective>{}), Parser<AccClauseList>{}))
 
-TYPE_PARSER(
-    construct<OpenACCLoopConstruct>(sourced(Parser<AccBeginLoopDirective>{})))
+TYPE_PARSER(construct<AccEndLoop>(startAccLine >> "END LOOP"_tok))
+
+TYPE_PARSER(construct<OpenACCLoopConstruct>(
+    sourced(Parser<AccBeginLoopDirective>{} / endAccLine),
+    withMessage("A DO loop must follow the loop construct"_err_en_US,
+        Parser<DoConstruct>{}),
+    maybe(Parser<AccEndLoop>{} / endAccLine)))
 
 // 2.15.1 Routine directive
 TYPE_PARSER(sourced(construct<OpenACCRoutineConstruct>(verbatim("ROUTINE"_tok),

flang/lib/Parser/unparse.cpp

@@ -1947,7 +1947,7 @@ class UnparseVisitor {
     Walk(std::get<AccBeginLoopDirective>(x.t));
     Put("\n");
     EndOpenACC();
-    Walk(std::get<std::optional<DoConstruct>>(x.t));
+    Walk(std::get<DoConstruct>(x.t));
   }
   void Unparse(const AccBeginLoopDirective &x) {
     Walk(std::get<AccLoopDirective>(x.t));

flang/lib/Semantics/canonicalize-acc.cpp

@@ -52,7 +52,8 @@ class CanonicalizationOfAcc {
   // If there are n tile sizes in the list, the loop construct must be
   // immediately followed by n tightly-nested loops.
   template <typename C, typename D>
-  void CheckTileClauseRestriction(const C &x) {
+  void CheckTileClauseRestriction(
+      const C &x, const parser::DoConstruct &outer) {
     const auto &beginLoopDirective = std::get<D>(x.t);
     const auto &accClauseList =
         std::get<parser::AccClauseList>(beginLoopDirective.t);
@@ -63,11 +64,10 @@ class CanonicalizationOfAcc {
         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()) {
+        if (outer.IsDoConcurrent()) {
           return; // Tile is not allowed on DO CONCURRENT
         }
-        for (const parser::DoConstruct *loop{&*outer}; loop && tileArgNb > 0;
+        for (const parser::DoConstruct *loop{&outer}; loop && tileArgNb > 0;
              --tileArgNb) {
           const auto &block{std::get<parser::Block>(loop->t)};
           const auto it{block.begin()};
@@ -89,9 +89,9 @@ class CanonicalizationOfAcc {
   // 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()) {
+  void CheckDoConcurrentClauseRestriction(
+      const C &x, const parser::DoConstruct &doCons) {
+    if (!doCons.IsDoConcurrent()) {
       return;
     }
     const auto &beginLoopDirective = std::get<D>(x.t);
@@ -109,73 +109,36 @@ class CanonicalizationOfAcc {
 
   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
-    //
-    // After rewriting:
-    //   ExecutableConstruct -> OpenACCConstruct -> OpenACCLoopConstruct
-    //     AccBeginLoopDirective
-    //     DoConstruct
-    parser::Block::iterator nextIt;
     auto &beginDir{std::get<parser::AccBeginLoopDirective>(x.t)};
     auto &dir{std::get<parser::AccLoopDirective>(beginDir.t)};
+    const auto &doCons{std::get<parser::DoConstruct>(x.t)};
 
-    nextIt = it;
-    if (++nextIt != block.end()) {
-      if (auto *doCons{parser::Unwrap<parser::DoConstruct>(*nextIt)}) {
-        if (!doCons->GetLoopControl()) {
-          messages_.Say(dir.source,
-              "DO loop after the %s directive must have loop control"_err_en_US,
-              parser::ToUpperCaseLetters(dir.source.ToString()));
-          return;
-        }
-
-        // move DoConstruct
-        std::get<std::optional<parser::DoConstruct>>(x.t) = std::move(*doCons);
-        nextIt = block.erase(nextIt);
-
-        CheckDoConcurrentClauseRestriction<parser::OpenACCLoopConstruct,
-            parser::AccBeginLoopDirective>(x);
-        CheckTileClauseRestriction<parser::OpenACCLoopConstruct,
-            parser::AccBeginLoopDirective>(x);
-
-        return; // found do-loop
-      }
+    if (!doCons.GetLoopControl()) {
+      messages_.Say(dir.source,
+          "DO loop after the %s directive must have loop control"_err_en_US,
+          parser::ToUpperCaseLetters(dir.source.ToString()));
+      return;
     }
-    messages_.Say(dir.source,
-        "A DO loop must follow the %s directive"_err_en_US,
-        parser::ToUpperCaseLetters(dir.source.ToString()));
+
+    CheckDoConcurrentClauseRestriction<parser::OpenACCLoopConstruct,
+        parser::AccBeginLoopDirective>(x, doCons);
+    CheckTileClauseRestriction<parser::OpenACCLoopConstruct,
+        parser::AccBeginLoopDirective>(x, doCons);
   }
 
   void RewriteOpenACCCombinedConstruct(parser::OpenACCCombinedConstruct &x,
       parser::Block &block, parser::Block::iterator it) {
-    // Check the sequence of DoConstruct in the same iteration
-    //
-    // Original:
-    //   ExecutableConstruct -> OpenACCConstruct -> OpenACCCombinedConstruct
-    //     ACCBeginCombinedDirective
-    //   ExecutableConstruct -> DoConstruct
-    //   ExecutableConstruct -> AccEndCombinedDirective (if available)
-    //
-    // After rewriting:
-    //   ExecutableConstruct -> OpenACCConstruct -> OpenACCCombinedConstruct
-    //     ACCBeginCombinedDirective
-    //     DoConstruct
-    //     AccEndCombinedDirective (if available)
+    // Check the sequence of DoConstruct in the same iteration.
     parser::Block::iterator nextIt;
     auto &beginDir{std::get<parser::AccBeginCombinedDirective>(x.t)};
     auto &dir{std::get<parser::AccCombinedDirective>(beginDir.t)};
-    auto &doConstruct{std::get<std::optional<parser::DoConstruct>>(x.t)};
+    const auto &doConstruct{std::get<std::optional<parser::DoConstruct>>(x.t)};
 
     if (doConstruct) {
       CheckDoConcurrentClauseRestriction<parser::OpenACCCombinedConstruct,
-          parser::AccBeginCombinedDirective>(x);
+          parser::AccBeginCombinedDirective>(x, *doConstruct);
       CheckTileClauseRestriction<parser::OpenACCCombinedConstruct,
-          parser::AccBeginCombinedDirective>(x);
+          parser::AccBeginCombinedDirective>(x, *doConstruct);
       if (!doConstruct->GetLoopControl()) {
         messages_.Say(dir.source,
             "DO loop after the %s directive must have loop control"_err_en_US,

flang/lib/Semantics/resolve-directives.cpp

@@ -1088,8 +1088,8 @@ void AccAttributeVisitor::PrivatizeAssociatedLoopIndex(
     return nullptr;
   };
 
-  const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
-  for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
+  const auto &outer{std::get<parser::DoConstruct>(x.t)};
+  for (const parser::DoConstruct *loop{&outer}; loop && level > 0; --level) {
     // go through all the nested do-loops and resolve index variables
     const parser::Name *iv{GetLoopIndex(*loop)};
     if (iv) {

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

@@ -15,9 +15,7 @@ program openacc_clause_validity
   real(8) :: a(256)
   real(8) :: aa(256, 256)
 
-  !ERROR: A DO loop must follow the LOOP directive
-  !$acc loop
-  i = 1
+  i = 0
 
   !ERROR: DO loop after the LOOP directive must have loop control
   !$acc loop

flang/test/Semantics/OpenACC/acc-loop-validity.f90

@@ -0,0 +1,15 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenacc
+
+program openacc_clause_validity
+
+  implicit none
+
+  integer :: i
+
+  i = 0
+
+  !$acc loop
+  !ERROR: A DO loop must follow the loop construct
+  i = 1
+
+end

flang/test/Semantics/OpenACC/acc-loop.f90

@@ -263,4 +263,9 @@ program openacc_loop_validity
   do i = 1, N
   end do
 
+  !$acc loop
+  do i = 1, N
+  end do
+  !$acc end loop
+
 end program openacc_loop_validity