[flang-commits] [flang] [flang][OpenMP] Use OmpDirectiveSpecifications in helper functions (PR #190644)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

<details>
<summary>Changes</summary>

This will make them more reusable, for example when processing APPLY clause in the future.

Issue: https://github.com/llvm/llvm-project/issues/185287

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


2 Files Affected:

- (modified) flang/include/flang/Semantics/openmp-utils.h (+3-3) 
- (modified) flang/lib/Semantics/openmp-utils.cpp (+45-54) 


``````````diff
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index c29cc4e3a33e2..6a41ada9067c5 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -113,6 +113,9 @@ bool IsPointerAssignment(const evaluate::Assignment &x);
 
 MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
 
+bool IsLoopTransforming(llvm::omp::Directive dir);
+bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec);
+
 /// A representation of a "because" message.
 struct Reason {
   Reason() = default;
@@ -157,9 +160,6 @@ WithReason<int64_t> GetNumArgumentsWithReason(
     const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
     unsigned version);
 
-bool IsLoopTransforming(llvm::omp::Directive dir);
-bool IsFullUnroll(const parser::OpenMPLoopConstruct &x);
-
 // Return the depth of the affected nests:
 //   {affected-depth, reason, must-be-perfect-nest}.
 std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index e9548816fc665..33707f9d0d95b 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -554,6 +554,44 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
       instance.u);
 }
 
+bool IsLoopTransforming(llvm::omp::Directive dir) {
+  switch (dir) {
+  // TODO case llvm::omp::Directive::OMPD_flatten:
+  case llvm::omp::Directive::OMPD_fuse:
+  case llvm::omp::Directive::OMPD_interchange:
+  case llvm::omp::Directive::OMPD_nothing:
+  case llvm::omp::Directive::OMPD_reverse:
+  // TODO case llvm::omp::Directive::OMPD_split:
+  case llvm::omp::Directive::OMPD_stripe:
+  case llvm::omp::Directive::OMPD_tile:
+  case llvm::omp::Directive::OMPD_unroll:
+    return true;
+  default:
+    return false;
+  }
+}
+
+bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec) {
+  if (spec.DirId() == llvm::omp::Directive::OMPD_unroll) {
+    return !parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_partial);
+  }
+  return false;
+}
+
+static bool IsTransformableLoop(const parser::OmpDirectiveSpecification &spec) {
+  return !IsFullUnroll(spec) && IsLoopTransforming(spec.DirId());
+}
+
+static bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
+  if (auto *loop{parser::Unwrap<parser::DoConstruct>(epc)}) {
+    return loop->IsDoNormal();
+  }
+  if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(epc)}) {
+    return IsTransformableLoop(omp->BeginDir());
+  }
+  return false;
+}
+
 static const auto MsgNotValidAffectedLoop{
     "%s is not a valid affected loop"_because_en_US};
 static const auto MsgClauseAbsentAssume{
@@ -638,33 +676,6 @@ WithReason<int64_t> GetNumArgumentsWithReason(
   return {};
 }
 
-bool IsLoopTransforming(llvm::omp::Directive dir) {
-  switch (dir) {
-  // TODO case llvm::omp::Directive::OMPD_flatten:
-  case llvm::omp::Directive::OMPD_fuse:
-  case llvm::omp::Directive::OMPD_interchange:
-  case llvm::omp::Directive::OMPD_nothing:
-  case llvm::omp::Directive::OMPD_reverse:
-  // TODO case llvm::omp::Directive::OMPD_split:
-  case llvm::omp::Directive::OMPD_stripe:
-  case llvm::omp::Directive::OMPD_tile:
-  case llvm::omp::Directive::OMPD_unroll:
-    return true;
-  default:
-    return false;
-  }
-}
-
-bool IsFullUnroll(const parser::OpenMPLoopConstruct &x) {
-  const parser::OmpDirectiveSpecification &beginSpec{x.BeginDir()};
-
-  if (beginSpec.DirName().v == llvm::omp::Directive::OMPD_unroll) {
-    return parser::omp::FindClause(
-               beginSpec, llvm::omp::Clause::OMPC_partial) == nullptr;
-  }
-  return false;
-}
-
 namespace {
 // Helper class to check if a given evaluate::Expr is an array expression.
 // This does not check any proper subexpressions of the expression (except
@@ -812,27 +823,6 @@ bool IsTransparentInterveningCode(const parser::ExecutionPartConstruct &x) {
       parser::Unwrap<parser::ContinueStmt>(x);
 }
 
-bool IsTransformableLoop(const parser::DoConstruct &loop) {
-  return loop.IsDoNormal();
-}
-
-bool IsTransformableLoop(const parser::OpenMPLoopConstruct &omp) {
-  if (IsFullUnroll(omp)) {
-    return false;
-  }
-  return IsLoopTransforming(omp.BeginDir().DirId());
-}
-
-bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
-  if (auto *loop{parser::Unwrap<parser::DoConstruct>(epc)}) {
-    return IsTransformableLoop(*loop);
-  }
-  if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(epc)}) {
-    return IsTransformableLoop(*omp);
-  }
-  return false;
-}
-
 template <typename T,
     typename = std::enable_if_t<std::is_arithmetic_v<llvm::remove_cvref_t<T>>>>
 WithReason<T> operator+(const WithReason<T> &a, const WithReason<T> &b) {
@@ -1057,7 +1047,7 @@ LoopSequence::LoopSequence(
 std::unique_ptr<LoopSequence::Construct> LoopSequence::createConstructEntry(
     const parser::ExecutionPartConstruct &code) {
   if (auto *loop{parser::Unwrap<parser::DoConstruct>(code)}) {
-    if (allowAllLoops_ || IsTransformableLoop(*loop)) {
+    if (allowAllLoops_ || IsTransformableLoop(code)) {
       auto &body{std::get<parser::Block>(loop->t)};
       return std::make_unique<Construct>(body, &code);
     }
@@ -1126,7 +1116,7 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
   }
 
   // TODO: Handle split, apply.
-  if (IsFullUnroll(omp)) {
+  if (IsFullUnroll(beginSpec)) {
     return {};
   }
 
@@ -1250,10 +1240,10 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
   auto &omp{DEREF(parser::Unwrap<parser::OpenMPLoopConstruct>(*entry_->owner))};
   const parser::OmpDirectiveSpecification &beginSpec{omp.BeginDir()};
   llvm::omp::Directive dir{beginSpec.DirId()};
-  bool isFullUnroll{IsFullUnroll(omp)};
+  bool isFullUnroll{IsFullUnroll(beginSpec)};
 
   // Check full unroll separately.
-  if (!isFullUnroll && !IsTransformableLoop(omp)) {
+  if (!isFullUnroll && !IsTransformableLoop(beginSpec)) {
     Reason reason;
     reason.Say(beginSpec.DirName().source,
         "This construct is not a DO-loop or a loop-nest-generating construct"_because_en_US);
@@ -1368,10 +1358,11 @@ static Reason WhyNotWellFormed(
   Reason reason;
   parser::CharBlock source{*parser::GetSource(badCode)};
   if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(badCode)}) {
-    if (IsFullUnroll(*omp)) {
+    const parser::OmpDirectiveSpecification &beginSpec{omp->BeginDir()};
+    if (IsFullUnroll(beginSpec)) {
       reason.Say(source, MsgConstructDoesNotResult, "Fully unrolled loop",
           isSequence ? "a loop nest or a loop sequence" : "a loop nest");
-    } else if (!IsLoopTransforming(omp->BeginDir().DirId())) {
+    } else if (!IsLoopTransforming(beginSpec.DirId())) {
       reason.Say(source,
           "Only loop-transforming constructs are allowed inside loop constructs"_because_en_US);
     }

``````````

</details>


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