[flang-commits] [flang] [flang][OpenMP] Make sure that exec METADIRECTIVE is in execution part (PR #233147)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

<details>
<summary>Changes</summary>

A METADIRECTIVE can be in either the specification part or the execution part, depending on whether the variant directives contained in it are declarative or executable. The OmpMetadirectiveDirective AST node can hold any combination of variants, that is it can represent both kinds. When the parser builds the specification part and attempts to create an OmpMetadirectiveDirective, once it gets one it puts it in the specification part regardless of the contents.

When an executable METADIRECTIVE is the first statement of the execution part, it will end up in the specification part. This is an identical situation as that for utility directives.

Recognize executable METADIRECTIVEs at the end of the specification part and move them over to the beginning of the execution part.

---
Full diff: https://github.com/llvm/llvm-project/pull/233147.diff


2 Files Affected:

- (modified) flang/lib/Semantics/canonicalize-omp.cpp (+55-13) 
- (modified) flang/test/Parser/OpenMP/metadirective-dirspec.f90 (+4-2) 


``````````diff
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 14316c6e180ede..7ca63ae5b1d7e6 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -162,16 +162,18 @@ void CanonicalizationOfOmp::CanonicalizeAllocateDirectives(
   }
 }
 
+template <typename T> parser::ExecutionPartConstruct MakeExec(T &&x) {
+  return parser::ExecutionPartConstruct(parser::ExecutableConstruct(
+      common::Indirection(parser::OpenMPConstruct(std::move(x)))));
+}
+
 parser::ExecutionPartConstruct CanonicalizationOfOmp::EmbedInExec(
     parser::OmpAllocateDirective *alo, parser::ExecutionPartConstruct &&epc) {
   // Nest current epc inside the allocate directive.
   std::get<parser::Block>(alo->t).push_front(std::move(epc));
   // Set the new epc to be the ExecutionPartConstruct made from
   // the allocate directive.
-  parser::OpenMPConstruct opc(std::move(*alo));
-  common::Indirection<parser::OpenMPConstruct> ind(std::move(opc));
-  parser::ExecutableConstruct ec(std::move(ind));
-  return parser::ExecutionPartConstruct(std::move(ec));
+  return MakeExec(std::move(*alo));
 }
 
 void CanonicalizationOfOmp::MakeExecutableAllocateFromDecls(
@@ -226,7 +228,7 @@ void CanonicalizationOfOmp::MakeExecutableAllocateFromOmps(
   }
 }
 
-// Canonicalization of utility directives.
+// Canonicalization of utility and metadirectives.
 //
 // This addresses the issue of directives that appear at the boundary
 // between the specification and the execution parts, e.g.
@@ -280,6 +282,39 @@ void CanonicalizationOfOmp::CanonicalizeUtilityDirectives(
   }
 }
 
+static bool IsExecutableOrNothing(const parser::OmpDirectiveSpecification &x) {
+  llvm::omp::Directive dirId = x.DirId();
+  if (dirId == llvm::omp::Directive::OMPD_nothing) {
+    return true;
+  }
+  return llvm::omp::getDirectiveCategory(dirId) ==
+      llvm::omp::Category::Executable;
+}
+
+static bool IsExecutable(const parser::OmpMetadirectiveDirective &md) {
+  for (const parser::OmpClause &clause : md.v.Clauses().v) {
+    if (auto *other = parser::Unwrap<parser::OmpOtherwiseClause>(clause)) {
+      if (other->v.has_value() && !IsExecutableOrNothing(other->v->value())) {
+        return false;
+      }
+    } else if (auto *when = parser::Unwrap<parser::OmpWhenClause>(clause)) {
+      using OmpDirectiveSpecification =
+          std::optional<common::Indirection<parser::OmpDirectiveSpecification>>;
+      if (auto &s = std::get<OmpDirectiveSpecification>(when->t)) {
+        if (!IsExecutableOrNothing(s->value())) {
+          return false;
+        }
+      }
+    } else if (auto *dv =
+                   parser::Unwrap<parser::OmpDefaultVariantClause>(clause)) {
+      if (!IsExecutableOrNothing(dv->v.value())) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
 void CanonicalizationOfOmp::MoveUtilityDirectivesFromDecls(
     std::list<parser::DeclarationConstruct> &decls, parser::Block &block) {
   // Find the trailing range of DeclarationConstructs that are OpenMP
@@ -288,12 +323,16 @@ void CanonicalizationOfOmp::MoveUtilityDirectivesFromDecls(
     for (auto rit = decls.rbegin(), rend = decls.rend(); rit != rend; ++rit) {
       parser::DeclarationConstruct &dc = *rit;
       if (auto *odc = parser::Unwrap<parser::OpenMPDeclarativeConstruct>(dc)) {
-        if (!std::holds_alternative<parser::OmpUtilityDirective>(odc->u)) {
-          return rit;
+        if (std::holds_alternative<parser::OmpUtilityDirective>(odc->u)) {
+          continue;
+        }
+        if (auto *md = parser::Unwrap<parser::OmpMetadirectiveDirective>(dc)) {
+          if (IsExecutable(*md)) {
+            continue;
+          }
         }
-      } else {
-        return rit;
       }
+      return rit;
     }
     return decls.rend();
   }();
@@ -304,10 +343,13 @@ void CanonicalizationOfOmp::MoveUtilityDirectivesFromDecls(
         using OpenMPDeclarativeConstruct =
             common::Indirection<parser::OpenMPDeclarativeConstruct>;
         auto &oc = std::get<OpenMPDeclarativeConstruct>(sc.u).value();
-        auto &ut = std::get<parser::OmpUtilityDirective>(oc.u);
-
-        return parser::ExecutionPartConstruct(parser::ExecutableConstruct(
-            common::Indirection(parser::OpenMPConstruct(std::move(ut)))));
+        if (auto *x = std::get_if<parser::OmpMetadirectiveDirective>(&oc.u)) {
+          return MakeExec(parser::OpenMPStandaloneConstruct(std::move(*x)));
+        }
+        if (auto *x = std::get_if<parser::OmpUtilityDirective>(&oc.u)) {
+          return MakeExec(std::move(*x));
+        }
+        llvm_unreachable("");
       });
 
   decls.erase(rlast.base(), decls.end());
diff --git a/flang/test/Parser/OpenMP/metadirective-dirspec.f90 b/flang/test/Parser/OpenMP/metadirective-dirspec.f90
index d63db6c93cf404..16b4a406952378 100644
--- a/flang/test/Parser/OpenMP/metadirective-dirspec.f90
+++ b/flang/test/Parser/OpenMP/metadirective-dirspec.f90
@@ -40,7 +40,8 @@ subroutine f01(x)
 !UNPARSE: !$OMP METADIRECTIVE WHEN(USER={CONDITION(.true._4)}: CRITICAL(x))
 !UNPARSE: END SUBROUTINE
 
-!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpMetadirectiveDirective
+!PARSE-TREE:  OpenMPConstruct -> OpenMPStandaloneConstruct -> OmpMetadirectiveDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = metadirective
 !PARSE-TREE: | OmpClauseList -> OmpClause -> When -> OmpWhenClause
 !PARSE-TREE: | | Modifier -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | | OmpTraitSetSelectorName -> Value = User
@@ -227,7 +228,8 @@ subroutine f06(x, y)
 !UNPARSE: !$OMP METADIRECTIVE WHEN(USER={CONDITION(.true._4)}: FLUSH(x, y))
 !UNPARSE: END SUBROUTINE
 
-!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpMetadirectiveDirective
+!PARSE-TREE: OpenMPConstruct -> OpenMPStandaloneConstruct -> OmpMetadirectiveDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = metadirective
 !PARSE-TREE: | OmpClauseList -> OmpClause -> When -> OmpWhenClause
 !PARSE-TREE: | | Modifier -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | | OmpTraitSetSelectorName -> Value = User

``````````

</details>


https://github.com/llvm/llvm-project/pull/233147