[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering #110568

kparzysz · 2024-09-30T20:41:30Z

Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

llvmbot · 2024-09-30T20:42:06Z

@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

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

12 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+2)
(modified) flang/include/flang/Parser/parse-tree.h (+9)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+16-3)
(modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+2)
(modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+3-1)
(modified) flang/lib/Lower/OpenMP/Utils.cpp (+13)
(modified) flang/lib/Lower/OpenMP/Utils.h (+3)
(modified) flang/lib/Parser/openmp-parsers.cpp (+7-1)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+26-8)
(modified) flang/lib/Semantics/resolve-directives.cpp (+2-1)
(added) flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 (+12)
(modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1-1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 0b4ebcbaa24c40..bf00e6b43d0668 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -522,6 +522,8 @@ class ParseTreeDumper {
   NODE(parser, OmpEndSectionsDirective)
   NODE(parser, OmpIfClause)
   NODE_ENUM(OmpIfClause, DirectiveNameModifier)
+  NODE_ENUM(OmpLastprivateClause, LastprivateModifier)
+  NODE(parser, OmpLastprivateClause)
   NODE(parser, OmpLinearClause)
   NODE(OmpLinearClause, WithModifier)
   NODE(OmpLinearClause, WithoutModifier)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 548fcc81984b2a..7057ac2267aa15 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3648,6 +3648,15 @@ struct OmpAtomicDefaultMemOrderClause {
       OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType);
 };
 
+// OMP 5.0 2.19.4.5 lastprivate-clause ->
+//                    LASTPRIVATE ([lastprivate-modifier :] list)
+//                  lastprivate-modifier -> CONDITIONAL
+struct OmpLastprivateClause {
+  TUPLE_CLASS_BOILERPLATE(OmpLastprivateClause);
+  ENUM_CLASS(LastprivateModifier, Conditional);
+  std::tuple<std::optional<LastprivateModifier>, OmpObjectList> t;
+};
+
 // OpenMP Clauses
 struct OmpClause {
   UNION_CLASS_BOILERPLATE(OmpClause);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index efac7757ca5855..c93767eb8a5507 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -783,9 +783,22 @@ IsDevicePtr make(const parser::OmpClause::IsDevicePtr &inp,
 
 Lastprivate make(const parser::OmpClause::Lastprivate &inp,
                  semantics::SemanticsContext &semaCtx) {
-  // inp.v -> parser::OmpObjectList
-  return Lastprivate{{/*LastprivateModifier=*/std::nullopt,
-                      /*List=*/makeObjects(inp.v, semaCtx)}};
+  // inp.v -> parser::OmpLastprivateClause
+  using wrapped = parser::OmpLastprivateClause;
+
+  CLAUSET_ENUM_CONVERT( //
+      convert, parser::OmpLastprivateClause::LastprivateModifier,
+      Lastprivate::LastprivateModifier,
+      // clang-format off
+      MS(Conditional, Conditional)
+      // clang-format on
+  );
+
+  auto &t0 = std::get<std::optional<wrapped::LastprivateModifier>>(inp.v.t);
+  auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+
+  return Lastprivate{{/*LastprivateModifier=*/maybeApply(convert, t0),
+                      /*List=*/makeObjects(t1, semaCtx)}};
 }
 
 Linear make(const parser::OmpClause::Linear &inp,
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 5f4138e0f63e73..5701c54cfd4eac 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -151,6 +151,8 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
                                  explicitlyPrivatizedSymbols);
     } else if (const auto &lastPrivateClause =
                    std::get_if<omp::clause::Lastprivate>(&clause.u)) {
+      lastprivateModifierNotSupported(*lastPrivateClause,
+                                      converter.getCurrentLocation());
       const ObjectList &objects = std::get<ObjectList>(lastPrivateClause->t);
       collectOmpObjectListSymbol(objects, explicitlyPrivatizedSymbols);
     }
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d528772f28724b..393883708c7004 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1554,7 +1554,9 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
 
   for (const Clause &clause : item->clauses) {
     if (clause.id == llvm::omp::Clause::OMPC_lastprivate) {
-      lastprivates.push_back(&std::get<clause::Lastprivate>(clause.u));
+      auto &lastp = std::get<clause::Lastprivate>(clause.u);
+      lastprivateModifierNotSupported(lastp, converter.getCurrentLocation());
+      lastprivates.push_back(&lastp);
     } else {
       switch (clause.id) {
       case llvm::omp::Clause::OMPC_firstprivate:
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index 8073b24a1d5b45..47bc12e1b8a030 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -17,6 +17,7 @@
 #include <flang/Lower/ConvertType.h>
 #include <flang/Lower/PFTBuilder.h>
 #include <flang/Optimizer/Builder/FIRBuilder.h>
+#include <flang/Optimizer/Builder/Todo.h>
 #include <flang/Parser/parse-tree.h>
 #include <flang/Parser/tools.h>
 #include <flang/Semantics/tools.h>
@@ -356,6 +357,18 @@ semantics::Symbol *getOmpObjectSymbol(const parser::OmpObject &ompObject) {
   return sym;
 }
 
+void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp,
+                                     mlir::Location loc) {
+  using Lastprivate = omp::clause::Lastprivate;
+  auto &maybeMod =
+      std::get<std::optional<Lastprivate::LastprivateModifier>>(lastp.t);
+  if (maybeMod) {
+    assert(*maybeMod == Lastprivate::LastprivateModifier::Conditional &&
+           "Unexpected lastprivate modifier");
+    TODO(loc, "lastprivate clause with CONDITIONAL modifier");
+  }
+}
+
 } // namespace omp
 } // namespace lower
 } // namespace Fortran
diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h
index 0b4fe9044bfa7b..658d062e67b271 100644
--- a/flang/lib/Lower/OpenMP/Utils.h
+++ b/flang/lib/Lower/OpenMP/Utils.h
@@ -100,6 +100,9 @@ void genObjectList(const ObjectList &objects,
                    lower::AbstractConverter &converter,
                    llvm::SmallVectorImpl<mlir::Value> &operands);
 
+void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp,
+                                     mlir::Location loc);
+
 } // namespace omp
 } // namespace lower
 } // namespace Fortran
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52789d6e5f0f6b..d40ccbf140c884 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -232,6 +232,12 @@ TYPE_PARSER(construct<OmpOrderClause>(
 TYPE_PARSER(
     construct<OmpObject>(designator) || construct<OmpObject>("/" >> name / "/"))
 
+// OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list)
+TYPE_PARSER(construct<OmpLastprivateClause>(
+    maybe("CONDITIONAL" >>
+          pure(OmpLastprivateClause::LastprivateModifier::Conditional) / ":"),
+    Parser<OmpObjectList>{}))
+
 TYPE_PARSER(
     "ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
     "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
@@ -289,7 +295,7 @@ TYPE_PARSER(
     "IS_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::IsDevicePtr>(
                            parenthesized(Parser<OmpObjectList>{}))) ||
     "LASTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Lastprivate>(
-                         parenthesized(Parser<OmpObjectList>{}))) ||
+                         parenthesized(Parser<OmpLastprivateClause>{}))) ||
     "LINEAR" >> construct<OmpClause>(construct<OmpClause::Linear>(
                     parenthesized(Parser<OmpLinearClause>{}))) ||
     "LINK" >> construct<OmpClause>(construct<OmpClause::Link>(
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 51341b3faf3a45..d8f939d4cff34d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3174,11 +3174,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) {
 void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_lastprivate);
 
-  CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v, "LASTPRIVATE");
+  const auto &objectList{std::get<parser::OmpObjectList>(x.v.t)};
+  CheckIsVarPartOfAnotherVar(
+      GetContext().clauseSource, objectList, "LASTPRIVATE");
 
   DirectivesClauseTriple dirClauseTriple;
   SymbolSourceMap currSymbols;
-  GetSymbolsInObjectList(x.v, currSymbols);
+  GetSymbolsInObjectList(objectList, currSymbols);
   CheckDefinableObjects(currSymbols, GetClauseKindForParserClass(x));
   CheckCopyingPolymorphicAllocatable(
       currSymbols, llvm::omp::Clause::OMPC_lastprivate);
@@ -3193,6 +3195,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
 
   CheckPrivateSymbolsInOuterCxt(
       currSymbols, dirClauseTriple, GetClauseKindForParserClass(x));
+
+  using LastprivateModifier = parser::OmpLastprivateClause::LastprivateModifier;
+  const auto &maybeMod{std::get<std::optional<LastprivateModifier>>(x.v.t)};
+  if (maybeMod) {
+    unsigned version{context_.langOptions().OpenMPVersion};
+    unsigned allowedInVersion = 50;
+    if (version < allowedInVersion) {
+      std::string thisVersion{
+          std::to_string(version / 10) + "." + std::to_string(version % 10)};
+      context_.Say(GetContext().clauseSource,
+                   "LASTPRIVATE clause with CONDITIONAL modifier is not "
+                   "allowed in OpenMP v%s, try -fopenmp-version=%d"_err_en_US,
+                   thisVersion, allowedInVersion);
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
@@ -3620,15 +3637,16 @@ const parser::OmpObjectList *OmpStructureChecker::GetOmpObjectList(
   using MemberObjectListClauses =
       std::tuple<parser::OmpClause::Copyprivate, parser::OmpClause::Copyin,
           parser::OmpClause::Firstprivate, parser::OmpClause::From,
-          parser::OmpClause::Lastprivate, parser::OmpClause::Link,
-          parser::OmpClause::Private, parser::OmpClause::Shared,
-          parser::OmpClause::To, parser::OmpClause::Enter,
-          parser::OmpClause::UseDevicePtr, parser::OmpClause::UseDeviceAddr>;
+          parser::OmpClause::Link, parser::OmpClause::Private,
+          parser::OmpClause::Shared, parser::OmpClause::To,
+          parser::OmpClause::Enter, parser::OmpClause::UseDevicePtr,
+          parser::OmpClause::UseDeviceAddr>;
 
   // Clauses with OmpObjectList in the tuple
   using TupleObjectListClauses =
-      std::tuple<parser::OmpClause::Allocate, parser::OmpClause::Map,
-          parser::OmpClause::Reduction, parser::OmpClause::Aligned>;
+      std::tuple<parser::OmpClause::Allocate, parser::OmpClause::Lastprivate,
+          parser::OmpClause::Map, parser::OmpClause::Reduction,
+          parser::OmpClause::Aligned>;
 
   // TODO:: Generate the tuples using TableGen.
   // Handle other constructs with OmpObjectList such as OpenMPThreadprivate.
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 45fe17c0122c09..0c8e664abcfe3d 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -455,7 +455,8 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
     return false;
   }
   bool Pre(const parser::OmpClause::Lastprivate &x) {
-    ResolveOmpObjectList(x.v, Symbol::Flag::OmpLastPrivate);
+    const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
+    ResolveOmpObjectList(objList, Symbol::Flag::OmpLastPrivate);
     return false;
   }
   bool Pre(const parser::OmpClause::Copyin &x) {
diff --git a/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90
new file mode 100644
index 00000000000000..2b96093da3a8fa
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90
@@ -0,0 +1,12 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: lastprivate clause with CONDITIONAL modifier
+subroutine foo()
+  integer :: x, i
+  x = 1
+  !$omp parallel do lastprivate(conditional: x)
+  do i = 1, 100
+    x = x + 1
+  enddo
+end
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 01515108ca1857..fcf087d1f9c6e4 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -229,7 +229,7 @@ def OMPC_IsDevicePtr : Clause<"is_device_ptr"> {
 }
 def OMPC_LastPrivate : Clause<"lastprivate"> {
   let clangClass = "OMPLastprivateClause";
-  let flangClass = "OmpObjectList";
+  let flangClass = "OmpLastprivateClause";
 }
 def OMPC_Linear : Clause<"linear"> {
   let clangClass = "OMPLinearClause";

llvmbot · 2024-09-30T20:42:06Z

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

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

12 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+2)
(modified) flang/include/flang/Parser/parse-tree.h (+9)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+16-3)
(modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+2)
(modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+3-1)
(modified) flang/lib/Lower/OpenMP/Utils.cpp (+13)
(modified) flang/lib/Lower/OpenMP/Utils.h (+3)
(modified) flang/lib/Parser/openmp-parsers.cpp (+7-1)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+26-8)
(modified) flang/lib/Semantics/resolve-directives.cpp (+2-1)
(added) flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 (+12)
(modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1-1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 0b4ebcbaa24c40..bf00e6b43d0668 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -522,6 +522,8 @@ class ParseTreeDumper {
   NODE(parser, OmpEndSectionsDirective)
   NODE(parser, OmpIfClause)
   NODE_ENUM(OmpIfClause, DirectiveNameModifier)
+  NODE_ENUM(OmpLastprivateClause, LastprivateModifier)
+  NODE(parser, OmpLastprivateClause)
   NODE(parser, OmpLinearClause)
   NODE(OmpLinearClause, WithModifier)
   NODE(OmpLinearClause, WithoutModifier)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 548fcc81984b2a..7057ac2267aa15 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3648,6 +3648,15 @@ struct OmpAtomicDefaultMemOrderClause {
       OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType);
 };
 
+// OMP 5.0 2.19.4.5 lastprivate-clause ->
+//                    LASTPRIVATE ([lastprivate-modifier :] list)
+//                  lastprivate-modifier -> CONDITIONAL
+struct OmpLastprivateClause {
+  TUPLE_CLASS_BOILERPLATE(OmpLastprivateClause);
+  ENUM_CLASS(LastprivateModifier, Conditional);
+  std::tuple<std::optional<LastprivateModifier>, OmpObjectList> t;
+};
+
 // OpenMP Clauses
 struct OmpClause {
   UNION_CLASS_BOILERPLATE(OmpClause);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index efac7757ca5855..c93767eb8a5507 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -783,9 +783,22 @@ IsDevicePtr make(const parser::OmpClause::IsDevicePtr &inp,
 
 Lastprivate make(const parser::OmpClause::Lastprivate &inp,
                  semantics::SemanticsContext &semaCtx) {
-  // inp.v -> parser::OmpObjectList
-  return Lastprivate{{/*LastprivateModifier=*/std::nullopt,
-                      /*List=*/makeObjects(inp.v, semaCtx)}};
+  // inp.v -> parser::OmpLastprivateClause
+  using wrapped = parser::OmpLastprivateClause;
+
+  CLAUSET_ENUM_CONVERT( //
+      convert, parser::OmpLastprivateClause::LastprivateModifier,
+      Lastprivate::LastprivateModifier,
+      // clang-format off
+      MS(Conditional, Conditional)
+      // clang-format on
+  );
+
+  auto &t0 = std::get<std::optional<wrapped::LastprivateModifier>>(inp.v.t);
+  auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+
+  return Lastprivate{{/*LastprivateModifier=*/maybeApply(convert, t0),
+                      /*List=*/makeObjects(t1, semaCtx)}};
 }
 
 Linear make(const parser::OmpClause::Linear &inp,
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 5f4138e0f63e73..5701c54cfd4eac 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -151,6 +151,8 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
                                  explicitlyPrivatizedSymbols);
     } else if (const auto &lastPrivateClause =
                    std::get_if<omp::clause::Lastprivate>(&clause.u)) {
+      lastprivateModifierNotSupported(*lastPrivateClause,
+                                      converter.getCurrentLocation());
       const ObjectList &objects = std::get<ObjectList>(lastPrivateClause->t);
       collectOmpObjectListSymbol(objects, explicitlyPrivatizedSymbols);
     }
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d528772f28724b..393883708c7004 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1554,7 +1554,9 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
 
   for (const Clause &clause : item->clauses) {
     if (clause.id == llvm::omp::Clause::OMPC_lastprivate) {
-      lastprivates.push_back(&std::get<clause::Lastprivate>(clause.u));
+      auto &lastp = std::get<clause::Lastprivate>(clause.u);
+      lastprivateModifierNotSupported(lastp, converter.getCurrentLocation());
+      lastprivates.push_back(&lastp);
     } else {
       switch (clause.id) {
       case llvm::omp::Clause::OMPC_firstprivate:
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index 8073b24a1d5b45..47bc12e1b8a030 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -17,6 +17,7 @@
 #include <flang/Lower/ConvertType.h>
 #include <flang/Lower/PFTBuilder.h>
 #include <flang/Optimizer/Builder/FIRBuilder.h>
+#include <flang/Optimizer/Builder/Todo.h>
 #include <flang/Parser/parse-tree.h>
 #include <flang/Parser/tools.h>
 #include <flang/Semantics/tools.h>
@@ -356,6 +357,18 @@ semantics::Symbol *getOmpObjectSymbol(const parser::OmpObject &ompObject) {
   return sym;
 }
 
+void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp,
+                                     mlir::Location loc) {
+  using Lastprivate = omp::clause::Lastprivate;
+  auto &maybeMod =
+      std::get<std::optional<Lastprivate::LastprivateModifier>>(lastp.t);
+  if (maybeMod) {
+    assert(*maybeMod == Lastprivate::LastprivateModifier::Conditional &&
+           "Unexpected lastprivate modifier");
+    TODO(loc, "lastprivate clause with CONDITIONAL modifier");
+  }
+}
+
 } // namespace omp
 } // namespace lower
 } // namespace Fortran
diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h
index 0b4fe9044bfa7b..658d062e67b271 100644
--- a/flang/lib/Lower/OpenMP/Utils.h
+++ b/flang/lib/Lower/OpenMP/Utils.h
@@ -100,6 +100,9 @@ void genObjectList(const ObjectList &objects,
                    lower::AbstractConverter &converter,
                    llvm::SmallVectorImpl<mlir::Value> &operands);
 
+void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp,
+                                     mlir::Location loc);
+
 } // namespace omp
 } // namespace lower
 } // namespace Fortran
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52789d6e5f0f6b..d40ccbf140c884 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -232,6 +232,12 @@ TYPE_PARSER(construct<OmpOrderClause>(
 TYPE_PARSER(
     construct<OmpObject>(designator) || construct<OmpObject>("/" >> name / "/"))
 
+// OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list)
+TYPE_PARSER(construct<OmpLastprivateClause>(
+    maybe("CONDITIONAL" >>
+          pure(OmpLastprivateClause::LastprivateModifier::Conditional) / ":"),
+    Parser<OmpObjectList>{}))
+
 TYPE_PARSER(
     "ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
     "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
@@ -289,7 +295,7 @@ TYPE_PARSER(
     "IS_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::IsDevicePtr>(
                            parenthesized(Parser<OmpObjectList>{}))) ||
     "LASTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Lastprivate>(
-                         parenthesized(Parser<OmpObjectList>{}))) ||
+                         parenthesized(Parser<OmpLastprivateClause>{}))) ||
     "LINEAR" >> construct<OmpClause>(construct<OmpClause::Linear>(
                     parenthesized(Parser<OmpLinearClause>{}))) ||
     "LINK" >> construct<OmpClause>(construct<OmpClause::Link>(
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 51341b3faf3a45..d8f939d4cff34d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3174,11 +3174,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) {
 void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_lastprivate);
 
-  CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v, "LASTPRIVATE");
+  const auto &objectList{std::get<parser::OmpObjectList>(x.v.t)};
+  CheckIsVarPartOfAnotherVar(
+      GetContext().clauseSource, objectList, "LASTPRIVATE");
 
   DirectivesClauseTriple dirClauseTriple;
   SymbolSourceMap currSymbols;
-  GetSymbolsInObjectList(x.v, currSymbols);
+  GetSymbolsInObjectList(objectList, currSymbols);
   CheckDefinableObjects(currSymbols, GetClauseKindForParserClass(x));
   CheckCopyingPolymorphicAllocatable(
       currSymbols, llvm::omp::Clause::OMPC_lastprivate);
@@ -3193,6 +3195,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
 
   CheckPrivateSymbolsInOuterCxt(
       currSymbols, dirClauseTriple, GetClauseKindForParserClass(x));
+
+  using LastprivateModifier = parser::OmpLastprivateClause::LastprivateModifier;
+  const auto &maybeMod{std::get<std::optional<LastprivateModifier>>(x.v.t)};
+  if (maybeMod) {
+    unsigned version{context_.langOptions().OpenMPVersion};
+    unsigned allowedInVersion = 50;
+    if (version < allowedInVersion) {
+      std::string thisVersion{
+          std::to_string(version / 10) + "." + std::to_string(version % 10)};
+      context_.Say(GetContext().clauseSource,
+                   "LASTPRIVATE clause with CONDITIONAL modifier is not "
+                   "allowed in OpenMP v%s, try -fopenmp-version=%d"_err_en_US,
+                   thisVersion, allowedInVersion);
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
@@ -3620,15 +3637,16 @@ const parser::OmpObjectList *OmpStructureChecker::GetOmpObjectList(
   using MemberObjectListClauses =
       std::tuple<parser::OmpClause::Copyprivate, parser::OmpClause::Copyin,
           parser::OmpClause::Firstprivate, parser::OmpClause::From,
-          parser::OmpClause::Lastprivate, parser::OmpClause::Link,
-          parser::OmpClause::Private, parser::OmpClause::Shared,
-          parser::OmpClause::To, parser::OmpClause::Enter,
-          parser::OmpClause::UseDevicePtr, parser::OmpClause::UseDeviceAddr>;
+          parser::OmpClause::Link, parser::OmpClause::Private,
+          parser::OmpClause::Shared, parser::OmpClause::To,
+          parser::OmpClause::Enter, parser::OmpClause::UseDevicePtr,
+          parser::OmpClause::UseDeviceAddr>;
 
   // Clauses with OmpObjectList in the tuple
   using TupleObjectListClauses =
-      std::tuple<parser::OmpClause::Allocate, parser::OmpClause::Map,
-          parser::OmpClause::Reduction, parser::OmpClause::Aligned>;
+      std::tuple<parser::OmpClause::Allocate, parser::OmpClause::Lastprivate,
+          parser::OmpClause::Map, parser::OmpClause::Reduction,
+          parser::OmpClause::Aligned>;
 
   // TODO:: Generate the tuples using TableGen.
   // Handle other constructs with OmpObjectList such as OpenMPThreadprivate.
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 45fe17c0122c09..0c8e664abcfe3d 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -455,7 +455,8 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
     return false;
   }
   bool Pre(const parser::OmpClause::Lastprivate &x) {
-    ResolveOmpObjectList(x.v, Symbol::Flag::OmpLastPrivate);
+    const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
+    ResolveOmpObjectList(objList, Symbol::Flag::OmpLastPrivate);
     return false;
   }
   bool Pre(const parser::OmpClause::Copyin &x) {
diff --git a/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90
new file mode 100644
index 00000000000000..2b96093da3a8fa
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90
@@ -0,0 +1,12 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: lastprivate clause with CONDITIONAL modifier
+subroutine foo()
+  integer :: x, i
+  x = 1
+  !$omp parallel do lastprivate(conditional: x)
+  do i = 1, 100
+    x = x + 1
+  enddo
+end
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 01515108ca1857..fcf087d1f9c6e4 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -229,7 +229,7 @@ def OMPC_IsDevicePtr : Clause<"is_device_ptr"> {
 }
 def OMPC_LastPrivate : Clause<"lastprivate"> {
   let clangClass = "OMPLastprivateClause";
-  let flangClass = "OmpObjectList";
+  let flangClass = "OmpLastprivateClause";
 }
 def OMPC_Linear : Clause<"linear"> {
   let clangClass = "OMPLinearClause";

github-actions · 2024-09-30T20:45:07Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kiranchandramohan · 2024-10-01T13:19:48Z

Thanks @kparzysz for picking this up. Could you add an unparse or debug dump parse tree test as well?

kiranchandramohan

LGTM.

…m#110568) Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering

14440f1

Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

kparzysz requested review from ergawy, skatrak and kiranchandramohan September 30, 2024 20:41

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics flang:parser clang:openmp OpenMP related changes to Clang labels Sep 30, 2024

kparzysz mentioned this pull request Sep 30, 2024

[Flang][OpenMP] Important issues to be fixed prior to removing experimental status of OpenMP #110008

Open

58 tasks

formatting

24d66d5

kiranchandramohan approved these changes Oct 1, 2024

View reviewed changes

add missing unparse code, plus testcase for unparse and dump parse tree

04956d1

kparzysz merged commit f982443 into llvm:main Oct 2, 2024
8 checks passed

kparzysz deleted the users/kparzysz/flang-lastp-cond branch October 2, 2024 20:36

xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering (llv…

e7fcb6d

…m#110568) Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering #110568

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering #110568

kparzysz commented Sep 30, 2024

llvmbot commented Sep 30, 2024 •

edited

Loading

llvmbot commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 •

edited

Loading

kiranchandramohan commented Oct 1, 2024

kiranchandramohan left a comment

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering #110568

[flang][OpenMP] Parse lastprivate modifier, add TODO to lowering #110568

Conversation

kparzysz commented Sep 30, 2024

llvmbot commented Sep 30, 2024 • edited Loading

llvmbot commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 • edited Loading

kiranchandramohan commented Oct 1, 2024

kiranchandramohan left a comment

Choose a reason for hiding this comment

llvmbot commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024 •

edited

Loading