[flang-commits] [flang] [Flang] Add perfect-nest and rectangular-loop semantic tests (PR #160283)

Michael Kruse via flang-commits flang-commits at lists.llvm.org
Wed Sep 24 02:13:17 PDT 2025


https://github.com/Meinersbur updated https://github.com/llvm/llvm-project/pull/160283

>From 3a141d6729e26a7eab821b86eee240ab4bfa322f Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 23 Sep 2025 12:59:14 +0200
Subject: [PATCH 1/2] Add perfect-nest and rectangular loop nest tests

---
 flang/lib/Semantics/resolve-directives.cpp | 146 ++++++++++++++++++++-
 flang/test/Semantics/OpenMP/do08.f90       |   1 +
 flang/test/Semantics/OpenMP/do13.f90       |   1 +
 flang/test/Semantics/OpenMP/do22.f90       |  73 +++++++++++
 4 files changed, 215 insertions(+), 6 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/do22.f90

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2d1bec9968593..5f2c9f676099c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -149,6 +149,9 @@ template <typename T> class DirectiveAttributeVisitor {
     dataSharingAttributeObjects_.clear();
   }
   bool HasDataSharingAttributeObject(const Symbol &);
+  std::tuple<const parser::Name *, const parser::ScalarExpr *,
+      const parser::ScalarExpr *, const parser::ScalarExpr *>
+  GetLoopBounds(const parser::DoConstruct &);
   const parser::Name *GetLoopIndex(const parser::DoConstruct &);
   const parser::DoConstruct *GetDoConstructIf(
       const parser::ExecutionPartConstruct &);
@@ -933,6 +936,13 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
     privateDataSharingAttributeObjects_.clear();
   }
 
+  /// Check that loops in the loop nest are perfectly nested, as well that lower
+  /// bound, upper bound, and step expressions do not use the iv
+  /// of a surrounding loop of the associated loops nest.
+  /// We do not support non-perfectly nested loops not non-rectangular loops yet
+  /// (both introduced in OpenMP 5.0)
+  void CheckPerfectNestAndRectangularLoop(const parser::OpenMPLoopConstruct &x);
+
   // Predetermined DSA rules
   void PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
       const parser::OpenMPLoopConstruct &);
@@ -1009,14 +1019,15 @@ bool DirectiveAttributeVisitor<T>::HasDataSharingAttributeObject(
 }
 
 template <typename T>
-const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
-    const parser::DoConstruct &x) {
+std::tuple<const parser::Name *, const parser::ScalarExpr *,
+    const parser::ScalarExpr *, const parser::ScalarExpr *>
+DirectiveAttributeVisitor<T>::GetLoopBounds(const parser::DoConstruct &x) {
   using Bounds = parser::LoopControl::Bounds;
   if (x.GetLoopControl()) {
     if (const Bounds * b{std::get_if<Bounds>(&x.GetLoopControl()->u)}) {
-      return &b->name.thing;
-    } else {
-      return nullptr;
+      auto &&step = b->step;
+      return {&b->name.thing, &b->lower, &b->upper,
+          step.has_value() ? &step.value() : nullptr};
     }
   } else {
     context_
@@ -1024,8 +1035,15 @@ const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
             "Loop control is not present in the DO LOOP"_err_en_US)
         .Attach(GetContext().directiveSource,
             "associated with the enclosing LOOP construct"_en_US);
-    return nullptr;
   }
+  return {nullptr, nullptr, nullptr, nullptr};
+}
+
+template <typename T>
+const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
+    const parser::DoConstruct &x) {
+  auto &&[iv, lb, ub, step] = GetLoopBounds(x);
+  return iv;
 }
 
 template <typename T>
@@ -1957,6 +1975,7 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
       }
     }
   }
+  CheckPerfectNestAndRectangularLoop(x);
   PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
   ordCollapseLevel = GetNumAffectedLoopsFromLoopConstruct(x) + 1;
   return true;
@@ -2152,6 +2171,121 @@ void OmpAttributeVisitor::CollectNumAffectedLoopsFromClauses(
   }
 }
 
+void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
+    const parser::OpenMPLoopConstruct
+        &x) { // GetAssociatedLoopLevelFromClauses(clauseList);
+  auto &&dirContext = GetContext();
+  std::int64_t dirDepth{dirContext.associatedLoopLevel};
+  if (dirDepth <= 0)
+    return;
+
+  Symbol::Flag ivDSA;
+  if (!llvm::omp::allSimdSet.test(GetContext().directive)) {
+    ivDSA = Symbol::Flag::OmpPrivate;
+  } else if (dirDepth == 1) {
+    ivDSA = Symbol::Flag::OmpLinear;
+  } else {
+    ivDSA = Symbol::Flag::OmpLastPrivate;
+  }
+
+  auto checkExprHasSymbols = [&](llvm::SmallVector<Symbol *> &ivs,
+                                 const parser::ScalarExpr *bound) {
+    if (ivs.empty())
+      return;
+
+    if (auto boundExpr{semantics::AnalyzeExpr(context_, *bound)}) {
+      semantics::UnorderedSymbolSet boundSyms =
+          evaluate::CollectSymbols(*boundExpr);
+      for (auto iv : ivs) {
+        if (boundSyms.count(*iv) != 0) {
+          // TODO: Point to occurence of iv in boundExpr, directiveSource as a
+          // note
+          context_.Say(dirContext.directiveSource,
+              "Trip count must be computable and invariant"_err_en_US);
+        }
+      }
+    }
+  };
+
+  // Skip over loop transformation directives
+  const parser::OpenMPLoopConstruct *innerMostLoop = &x;
+  const parser::NestedConstruct *innerMostNest = nullptr;
+  while (auto &optLoopCons{
+      std::get<std::optional<parser::NestedConstruct>>(innerMostLoop->t)}) {
+    innerMostNest = &(optLoopCons.value());
+    if (const auto *innerLoop{
+            std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
+                innerMostNest)}) {
+      innerMostLoop = &(innerLoop->value());
+    } else
+      break;
+  }
+
+  if (!innerMostNest)
+    return;
+  const auto &outer{std::get_if<parser::DoConstruct>(innerMostNest)};
+  if (!outer)
+    return;
+
+  llvm::SmallVector<Symbol *> ivs;
+  int curLevel = 0;
+  const parser::DoConstruct *loop{outer};
+  while (true) {
+    auto [iv, lb, ub, step] = GetLoopBounds(*loop);
+
+    if (lb)
+      checkExprHasSymbols(ivs, lb);
+    if (ub)
+      checkExprHasSymbols(ivs, ub);
+    if (step)
+      checkExprHasSymbols(ivs, step);
+    if (iv) {
+      if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())})
+        ivs.push_back(symbol);
+    }
+
+    // Stop after processing all affected loops
+    if (curLevel + 1 >= dirDepth)
+      break;
+
+    // Recurse into nested loop
+    const auto &block{std::get<parser::Block>(loop->t)};
+    if (block.empty()) {
+      // Insufficient number of nested loops already reported by
+      // CheckAssocLoopLevel()
+      break;
+    }
+
+    loop = GetDoConstructIf(block.front());
+    if (!loop) {
+      // Insufficient number of nested loops already reported by
+      // CheckAssocLoopLevel()
+      break;
+    }
+
+    auto checkPerfectNest = [&, this]() {
+      auto blockSize = block.size();
+      if (blockSize <= 1)
+        return;
+
+      if (parser::Unwrap<parser::ContinueStmt>(x))
+        blockSize -= 1;
+
+      if (blockSize <= 1)
+        return;
+
+      // Non-perfectly nested loop
+      // TODO: Point to non-DO statement, directiveSource as a note
+      context_.Say(dirContext.directiveSource,
+          "Canonical loop nest must be perfectly nested."_err_en_US);
+    };
+
+    checkPerfectNest();
+
+    ++curLevel;
+  }
+}
+
 // 2.15.1.1 Data-sharing Attribute Rules - Predetermined
 //   - The loop iteration variable(s) in the associated do-loop(s) of a do,
 //     parallel do, taskloop, or distribute construct is (are) private.
diff --git a/flang/test/Semantics/OpenMP/do08.f90 b/flang/test/Semantics/OpenMP/do08.f90
index 5143dff0dd315..bb3c1d0cd3855 100644
--- a/flang/test/Semantics/OpenMP/do08.f90
+++ b/flang/test/Semantics/OpenMP/do08.f90
@@ -61,6 +61,7 @@ program omp
   !$omp end do
 
 
+  !ERROR: Canonical loop nest must be perfectly nested.
   !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
   !$omp do collapse(3)
   do 60 i=2,200,2
diff --git a/flang/test/Semantics/OpenMP/do13.f90 b/flang/test/Semantics/OpenMP/do13.f90
index 6e9d1dddade4c..8f7844f4136f9 100644
--- a/flang/test/Semantics/OpenMP/do13.f90
+++ b/flang/test/Semantics/OpenMP/do13.f90
@@ -59,6 +59,7 @@ program omp
   !$omp end do
 
 
+  !ERROR: Canonical loop nest must be perfectly nested.
   !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
   !$omp do collapse(3)
   do 60 i=1,10
diff --git a/flang/test/Semantics/OpenMP/do22.f90 b/flang/test/Semantics/OpenMP/do22.f90
new file mode 100644
index 0000000000000..9d96d3af54e5c
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/do22.f90
@@ -0,0 +1,73 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Check for existence of loop following a DO directive
+
+subroutine do_imperfectly_nested_before
+  integer i, j
+
+  !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
+  !$omp do collapse(2)
+  do i = 1, 10
+    print *, i
+    do j = 1, 10
+      print *, i, j
+    end do
+  end do
+  !$omp end do
+end subroutine
+
+
+subroutine do_imperfectly_nested_behind
+  integer i, j
+
+  !ERROR: Canonical loop nest must be perfectly nested.
+  !$omp do collapse(2)
+  do i = 1, 10
+    do j = 1, 10
+      print *, i, j
+    end do
+    print *, i
+  end do
+  !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_lb
+  integer i, j
+
+  !ERROR: Trip count must be computable and invariant
+  !$omp do collapse(2)
+  do i = 1, 10
+    do j = i, 10
+      print *, i, j
+    end do
+  end do
+  !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_ub
+  integer i, j
+
+  !ERROR: Trip count must be computable and invariant
+  !$omp do collapse(2)
+  do i = 1, 10
+    do j = 0, i
+      print *, i, j
+    end do
+  end do
+  !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_step
+  integer i, j
+
+  !ERROR: Trip count must be computable and invariant
+  !$omp do collapse(2)
+  do i = 1, 10
+    do j = 1, 10, i
+      print *, i, j
+    end do
+  end do
+  !$omp end do
+end subroutine

>From 493442453cbac2366aa89abde361f7badaad4948 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 23 Sep 2025 16:39:44 +0200
Subject: [PATCH 2/2] Fix symbol resolution

---
 flang/lib/Semantics/resolve-directives.cpp | 48 ++++++++++------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 5f2c9f676099c..a0109324e546c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1975,7 +1975,10 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
       }
     }
   }
+
+  // Must be done before iv privatization
   CheckPerfectNestAndRectangularLoop(x);
+
   PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
   ordCollapseLevel = GetNumAffectedLoopsFromLoopConstruct(x) + 1;
   return true;
@@ -2172,37 +2175,29 @@ void OmpAttributeVisitor::CollectNumAffectedLoopsFromClauses(
 }
 
 void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
-    const parser::OpenMPLoopConstruct
-        &x) { // GetAssociatedLoopLevelFromClauses(clauseList);
-  auto &&dirContext = GetContext();
+    const parser::OpenMPLoopConstruct &x) {
+  auto &dirContext = GetContext();
   std::int64_t dirDepth{dirContext.associatedLoopLevel};
   if (dirDepth <= 0)
     return;
 
-  Symbol::Flag ivDSA;
-  if (!llvm::omp::allSimdSet.test(GetContext().directive)) {
-    ivDSA = Symbol::Flag::OmpPrivate;
-  } else if (dirDepth == 1) {
-    ivDSA = Symbol::Flag::OmpLinear;
-  } else {
-    ivDSA = Symbol::Flag::OmpLastPrivate;
-  }
-
   auto checkExprHasSymbols = [&](llvm::SmallVector<Symbol *> &ivs,
                                  const parser::ScalarExpr *bound) {
     if (ivs.empty())
       return;
-
-    if (auto boundExpr{semantics::AnalyzeExpr(context_, *bound)}) {
-      semantics::UnorderedSymbolSet boundSyms =
-          evaluate::CollectSymbols(*boundExpr);
-      for (auto iv : ivs) {
-        if (boundSyms.count(*iv) != 0) {
-          // TODO: Point to occurence of iv in boundExpr, directiveSource as a
-          // note
-          context_.Say(dirContext.directiveSource,
-              "Trip count must be computable and invariant"_err_en_US);
-        }
+    auto boundExpr{semantics::AnalyzeExpr(context_, *bound)};
+    if (!boundExpr)
+      return;
+    semantics::UnorderedSymbolSet boundSyms =
+        evaluate::CollectSymbols(*boundExpr);
+    if (boundSyms.empty())
+      return;
+    for (Symbol *iv : ivs) {
+      if (boundSyms.count(*iv) != 0) {
+        // TODO: Point to occurence of iv in boundExpr, directiveSource as a
+        //       note
+        context_.Say(dirContext.directiveSource,
+            "Trip count must be computable and invariant"_err_en_US);
       }
     }
   };
@@ -2217,8 +2212,9 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
             std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
                 innerMostNest)}) {
       innerMostLoop = &(innerLoop->value());
-    } else
+    } else {
       break;
+    }
   }
 
   if (!innerMostNest)
@@ -2228,7 +2224,7 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
     return;
 
   llvm::SmallVector<Symbol *> ivs;
-  int curLevel = 0;
+  int curLevel{0};
   const parser::DoConstruct *loop{outer};
   while (true) {
     auto [iv, lb, ub, step] = GetLoopBounds(*loop);
@@ -2240,7 +2236,7 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
     if (step)
       checkExprHasSymbols(ivs, step);
     if (iv) {
-      if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())})
+      if (auto *symbol{currScope().FindSymbol(iv->source)})
         ivs.push_back(symbol);
     }
 



More information about the flang-commits mailing list