[llvm-branch-commits] [flang] [flang][OpenMP] Convert repeatable clauses (except Map) in ClauseProc… (PR #81623)

[Krzysztof Parzyszek via llvm-branch-commits]

================
@@ -2865,53 +2865,45 @@ bool ClauseProcessor::processDepend(
     llvm::SmallVectorImpl<mlir::Value> &dependOperands) const {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
 
-  return findRepeatableClause<ClauseTy::Depend>(
-      [&](const ClauseTy::Depend *dependClause,
+  return findRepeatableClause<omp::clause::Depend>(
+      [&](const omp::clause::Depend &clause,
           const Fortran::parser::CharBlock &) {
-        const std::list<Fortran::parser::Designator> &depVal =
-            std::get<std::list<Fortran::parser::Designator>>(
-                std::get<Fortran::parser::OmpDependClause::InOut>(
-                    dependClause->v.u)
-                    .t);
+        assert(std::holds_alternative<omp::clause::Depend::InOut>(clause.u) &&
+               "Only InOut is handled at the moment");
+        const auto &inOut = std::get<omp::clause::Depend::InOut>(clause.u);
+        const auto &objects = std::get<omp::ObjectList>(inOut.t);
+
         mlir::omp::ClauseTaskDependAttr dependTypeOperand =
-            genDependKindAttr(firOpBuilder, dependClause);
-        dependTypeOperands.insert(dependTypeOperands.end(), depVal.size(),
-                                  dependTypeOperand);
-        for (const Fortran::parser::Designator &ompObject : depVal) {
-          Fortran::semantics::Symbol *sym = nullptr;
-          std::visit(
-              Fortran::common::visitors{
-                  [&](const Fortran::parser::DataRef &designator) {
-                    if (const Fortran::parser::Name *name =
-                            std::get_if<Fortran::parser::Name>(&designator.u)) {
-                      sym = name->symbol;
-                    } else if (std::get_if<Fortran::common::Indirection<
-                                   Fortran::parser::ArrayElement>>(
-                                   &designator.u)) {
-                      TODO(converter.getCurrentLocation(),
-                           "array sections not supported for task depend");
-                    }
-                  },
-                  [&](const Fortran::parser::Substring &designator) {
-                    TODO(converter.getCurrentLocation(),
-                         "substring not supported for task depend");
-                  }},
-              (ompObject).u);
+            genDependKindAttr(firOpBuilder, clause);
+        dependTypeOperands.append(objects.size(), dependTypeOperand);
+
+        for (const omp::Object &object : objects) {
+          assert(object.dsg && "Expecting designator");
+
+          if (Fortran::evaluate::ExtractSubstring(*object.dsg)) {
+            TODO(converter.getCurrentLocation(),
+                 "substring not supported for task depend");
+          } else if (Fortran::evaluate::IsArrayElement(*object.dsg)) {
+            TODO(converter.getCurrentLocation(),
+                 "array sections not supported for task depend");
+          }
+
+          Fortran::semantics::Symbol *sym = object.sym;
           const mlir::Value variable = converter.getSymbolAddress(*sym);
           dependOperands.push_back(variable);
         }
----------------
kparzysz wrote:

The old code is below for reference.

First, the code created (unconditionally) a list of designators from the list of objects.  In my implementation, if an object was a designator, the designator would be present in `omp::Object`.  If the object is not a designator, the code would fail at runtime in both cases.

Then, a `DataRef` object is obtained from the designator, if present, but that's only to get the symbol out of it.  The other two cases (Substring and ArrayElement) are both TODO, which is what my code does as well.

```
        const std::list<Fortran::parser::Designator> &depVal =
            std::get<std::list<Fortran::parser::Designator>>(
                std::get<Fortran::parser::OmpDependClause::InOut>(
                    dependClause->v.u)
                    .t);
        mlir::omp::ClauseTaskDependAttr dependTypeOperand =
            genDependKindAttr(firOpBuilder, dependClause);
        dependTypeOperands.insert(dependTypeOperands.end(), depVal.size(),
                                  dependTypeOperand);
        for (const Fortran::parser::Designator &ompObject : depVal) {
          Fortran::semantics::Symbol *sym = nullptr;
          std::visit(
              Fortran::common::visitors{
                  [&](const Fortran::parser::DataRef &designator) {
                    if (const Fortran::parser::Name *name =
                            std::get_if<Fortran::parser::Name>(&designator.u)) {
                      sym = name->symbol;
                    } else if (std::get_if<Fortran::common::Indirection<
                                   Fortran::parser::ArrayElement>>(
                                   &designator.u)) {
                      TODO(converter.getCurrentLocation(),
                           "array sections not supported for task depend");
                    }
                  },
                  [&](const Fortran::parser::Substring &designator) {
                    TODO(converter.getCurrentLocation(),
                         "substring not supported for task depend");
                  }},
              (ompObject).u);
          const mlir::Value variable = converter.getSymbolAddress(*sym);
```

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