Avoid attaching the sizes clause to the parent construct, instead find the tile

sizes through the parse tree when getting the information needed to create the
loop nest ops.

Author: jsjodin

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -46,6 +46,7 @@
 
 using namespace Fortran::lower::omp;
 using namespace Fortran::common::openmp;
+using namespace Fortran::semantics;
 
 //===----------------------------------------------------------------------===//
 // Code generation helper functions
@@ -1690,6 +1691,7 @@ genLoopNestClauses(lower::AbstractConverter &converter,
       int64_t collapseValue = evaluate::ToInt64(collapse.v).value();
       clauseOps.numCollapse = firOpBuilder.getI64IntegerAttr(collapseValue);
     } else if (clause.id == llvm::omp::Clause::OMPC_sizes) {
+      // This case handles the stand-alone tiling construct
       const auto &sizes = std::get<clause::Sizes>(clause.u);
       llvm::SmallVector<int64_t> sizeValues;
       for (auto &size : sizes.v) {
@@ -1699,6 +1701,12 @@ genLoopNestClauses(lower::AbstractConverter &converter,
       clauseOps.tileSizes = sizeValues;
     }
   }
+
+  llvm::SmallVector<int64_t> sizeValues;
+  auto *ompCons{eval.getIf<parser::OpenMPConstruct>()};
+  collectTileSizesFromOpenMPConstruct (ompCons, sizeValues, semaCtx);
+  if (sizeValues.size() > 0)
+    clauseOps.tileSizes = sizeValues;
 }
 
 static void genLoopClauses(
@@ -3936,21 +3944,6 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
   List<Clause> clauses = makeClauses(
       std::get<parser::OmpClauseList>(beginLoopDirective.t), semaCtx);
 
-  const auto &innerOptional =
-      std::get<std::optional<common::Indirection<parser::OpenMPLoopConstruct>>>(
-          loopConstruct.t);
-  if (innerOptional.has_value()) {
-    const auto &innerLoopDirective = innerOptional.value().value();
-    const auto &innerBegin =
-        std::get<parser::OmpBeginLoopDirective>(innerLoopDirective.t);
-    const auto &innerDirective =
-        std::get<parser::OmpLoopDirective>(innerBegin.t);
-    if (innerDirective.v == llvm::omp::Directive::OMPD_tile) {
-      clauses.append(
-          makeClauses(std::get<parser::OmpClauseList>(innerBegin.t), semaCtx));
-    }
-  }
-
   if (auto &endLoopDirective =
           std::get<std::optional<parser::OmpEndLoopDirective>>(
               loopConstruct.t)) {

flang/lib/Lower/OpenMP/Utils.cpp

@@ -14,6 +14,7 @@
 
 #include "ClauseFinder.h"
 #include "flang/Lower/OpenMP/Clauses.h"
+#include "flang/Evaluate/fold.h"
 #include <flang/Lower/AbstractConverter.h>
 #include <flang/Lower/ConvertType.h>
 #include <flang/Lower/DirectivesCommon.h>
@@ -24,10 +25,31 @@
 #include <flang/Parser/parse-tree.h>
 #include <flang/Parser/tools.h>
 #include <flang/Semantics/tools.h>
+#include <flang/Semantics/type.h>
 #include <llvm/Support/CommandLine.h>
 
 #include <iterator>
 
+using namespace Fortran::semantics;
+
+template <typename T>
+MaybeIntExpr
+EvaluateIntExpr(SemanticsContext &context, const T &expr) {
+  if (MaybeExpr maybeExpr{
+          Fold(context.foldingContext(), AnalyzeExpr(context, expr))}) {
+    if (auto *intExpr{Fortran::evaluate::UnwrapExpr<SomeIntExpr>(*maybeExpr)}) {
+      return std::move(*intExpr);
+    }
+  }
+  return std::nullopt;
+}
+
+template <typename T>
+std::optional<std::int64_t>
+EvaluateInt64(SemanticsContext &context, const T &expr) {
+  return Fortran::evaluate::ToInt64(EvaluateIntExpr(context, expr));
+}
+
 llvm::cl::opt<bool> treatIndexAsSection(
     "openmp-treat-index-as-section",
     llvm::cl::desc("In the OpenMP data clauses treat `a(N)` as `a(N:N)`."),
@@ -614,6 +636,43 @@ static void convertLoopBounds(lower::AbstractConverter &converter,
   }
 }
 
+// Populates the sizes vector with values if the given OpenMPConstruct
+// Contains a loop construct with an inner tiling construct.
+void collectTileSizesFromOpenMPConstruct(
+    const parser::OpenMPConstruct *ompCons,
+    llvm::SmallVectorImpl<int64_t> &tileSizes,
+    SemanticsContext &semaCtx) {
+  if (!ompCons)
+    return;
+
+  if (auto *ompLoop{std::get_if<parser::OpenMPLoopConstruct>(&ompCons->u)}) {
+    const auto &innerOptional = std::get<
+        std::optional<common::Indirection<parser::OpenMPLoopConstruct>>>(
+        ompLoop->t);
+    if (innerOptional.has_value()) {
+      const auto &innerLoopDirective = innerOptional.value().value();
+      const auto &innerBegin =
+          std::get<parser::OmpBeginLoopDirective>(innerLoopDirective.t);
+      const auto &innerDirective =
+          std::get<parser::OmpLoopDirective>(innerBegin.t).v;
+
+      if (innerDirective == llvm::omp::Directive::OMPD_tile) {
+        // Get the size values from parse tree and convert to a vector
+        const auto &innerClauseList{
+            std::get<parser::OmpClauseList>(innerBegin.t)};
+        for (const auto &clause : innerClauseList.v)
+          if (const auto tclause{
+                  std::get_if<parser::OmpClause::Sizes>(&clause.u)}) {
+            for (auto &tval : tclause->v) {
+              if (const auto v{EvaluateInt64(semaCtx, tval)})
+                tileSizes.push_back(*v);
+            }
+          }
+      }
+    }
+  }
+}
+
 bool collectLoopRelatedInfo(
     lower::AbstractConverter &converter, mlir::Location currentLocation,
     lower::pft::Evaluation &eval, const omp::List<omp::Clause> &clauses,
@@ -635,11 +694,34 @@ bool collectLoopRelatedInfo(
     collapseValue = evaluate::ToInt64(clause->v).value();
     found = true;
   }
+
+  // Collect sizes from tile directive if present
   std::int64_t sizesLengthValue = 0l;
-  if (auto *clause =
-          ClauseFinder::findUniqueClause<omp::clause::Sizes>(clauses)) {
-    sizesLengthValue = clause->v.size();
-    found = true;
+  if (auto *ompCons{eval.getIf<parser::OpenMPConstruct>()}) {
+    if (auto *ompLoop{std::get_if<parser::OpenMPLoopConstruct>(&ompCons->u)}) {
+      const auto &innerOptional = std::get<
+          std::optional<common::Indirection<parser::OpenMPLoopConstruct>>>(
+          ompLoop->t);
+      if (innerOptional.has_value()) {
+        const auto &innerLoopDirective = innerOptional.value().value();
+        const auto &innerBegin =
+            std::get<parser::OmpBeginLoopDirective>(innerLoopDirective.t);
+        const auto &innerDirective =
+            std::get<parser::OmpLoopDirective>(innerBegin.t).v;
+
+        if (innerDirective == llvm::omp::Directive::OMPD_tile) {
+          // Get the size values from parse tree and convert to a vector
+          const auto &innerClauseList{
+              std::get<parser::OmpClauseList>(innerBegin.t)};
+          for (const auto &clause : innerClauseList.v)
+            if (const auto tclause{
+                    std::get_if<parser::OmpClause::Sizes>(&clause.u)}) {
+              sizesLengthValue = tclause->v.size();
+              found = true;
+            }
+        }
+      }
+    }
   }
 
   collapseValue = collapseValue - sizesLengthValue;

flang/lib/Lower/OpenMP/Utils.h

@@ -167,6 +167,11 @@ bool collectLoopRelatedInfo(
     mlir::omp::LoopRelatedClauseOps &result,
     llvm::SmallVectorImpl<const semantics::Symbol *> &iv);
 
+void collectTileSizesFromOpenMPConstruct(
+    const parser::OpenMPConstruct *ompCons,
+    llvm::SmallVectorImpl<int64_t> &tileSizes,
+    Fortran::semantics::SemanticsContext &semaCtx);
+
 } // namespace omp
 } // namespace lower
 } // namespace Fortran

llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h

@@ -497,10 +497,6 @@ bool ConstructDecompositionT<C, H>::applyClause(
     if (llvm::omp::isAllowedClauseForDirective(last.id, node->id, version)) {
       last.clauses.push_back(node);
       return true;
-    } else {
-      //      llvm::errs() << "** OVERRIDING isAllowedClauseForDirective **\n";
-      last.clauses.push_back(node);
-      return true;
     }
   }