[Flang][OpenMP] Accept the reduction modifier #86492

kiranchandramohan · 2024-03-25T11:51:30Z

Accept the reduction modifier in the Flang parser. Issue a TODO message during lowering.

OpenMP 5.0 introduced the reduction modifier. Details can be seen in 2.19.5.4 reductionClause.
OpenMP 5.2 relevant section is 5.5.8reductionClause.

This will help remove some of the parser errors highlighted in the following post and also bring it to a well defined behaviour (produce TODO errors for unsupported features, do not crash).
https://discourse.llvm.org/t/proposal-rename-flang-new-to-flang/69462/60

github-actions · 2024-03-25T11:54:24Z

✅ With the latest revision this PR passed the Python code formatter.

github-actions · 2024-03-25T11:54:24Z

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

llvmbot · 2024-04-08T10:37:34Z

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

@llvm/pr-subscribers-flang-parser

Author: Kiran Chandramohan (kiranchandramohan)

Changes

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

10 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+1)
(modified) flang/include/flang/Parser/parse-tree.h (+4-1)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+21-5)
(modified) flang/lib/Lower/OpenMP/ReductionProcessor.cpp (+5)
(modified) flang/lib/Parser/openmp-parsers.cpp (+5)
(modified) flang/lib/Parser/unparse.cpp (+4)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+1-1)
(added) flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90 (+13)
(added) flang/test/Parser/OpenMP/reduction-modifier.f90 (+20)
(modified) llvm/include/llvm/Frontend/OpenMP/ClauseT.h (+1-1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 06c168a5de612c..477d391277ee25 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -542,6 +542,7 @@ class ParseTreeDumper {
   NODE_ENUM(OmpOrderModifier, Kind)
   NODE(parser, OmpProcBindClause)
   NODE_ENUM(OmpProcBindClause, Type)
+  NODE_ENUM(OmpReductionClause, ReductionModifier)
   NODE(parser, OmpReductionClause)
   NODE(parser, OmpInReductionClause)
   NODE(parser, OmpReductionCombiner)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 26b2e5f4e34b06..9e3c4e0e67c3b9 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3551,7 +3551,10 @@ struct OmpReductionOperator {
 //                                         variable-name-list)
 struct OmpReductionClause {
   TUPLE_CLASS_BOILERPLATE(OmpReductionClause);
-  std::tuple<OmpReductionOperator, OmpObjectList> t;
+  ENUM_CLASS(ReductionModifier, Inscan, Task, Default)
+  std::tuple<std::optional<ReductionModifier>, OmpReductionOperator,
+      OmpObjectList>
+      t;
 };
 
 // OMP 5.0 2.19.5.6 in_reduction-clause -> IN_REDUCTION (reduction-identifier:
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 40da71c8b55f80..97337cfc08c72a 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -1005,12 +1005,28 @@ ProcBind make(const parser::OmpClause::ProcBind &inp,
 Reduction make(const parser::OmpClause::Reduction &inp,
                semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpReductionClause
-  auto &t0 = std::get<parser::OmpReductionOperator>(inp.v.t);
-  auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+  using wrapped = parser::OmpReductionClause;
+
+  CLAUSET_ENUM_CONVERT( //
+      convert, wrapped::ReductionModifier, Reduction::ReductionModifier,
+      // clang-format off
+      MS(Inscan,  Inscan)
+      MS(Task,    Task)
+      MS(Default, Default)
+      // clang-format on
+  );
+
+  auto &t0 =
+      std::get<std::optional<parser::OmpReductionClause::ReductionModifier>>(
+          inp.v.t);
+  auto &t1 = std::get<parser::OmpReductionOperator>(inp.v.t);
+  auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
   return Reduction{
-      {/*ReductionIdentifiers=*/{makeReductionOperator(t0, semaCtx)},
-       /*ReductionModifier=*/std::nullopt,
-       /*List=*/makeObjects(t1, semaCtx)}};
+      {/*ReductionModifier=*/t0
+           ? std::make_optional<Reduction::ReductionModifier>(convert(*t0))
+           : std::nullopt,
+       /*ReductionIdentifiers=*/{makeReductionOperator(t1, semaCtx)},
+       /*List=*/makeObjects(t2, semaCtx)}};
 }
 
 // Relaxed: empty
diff --git a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
index c1c94119fd9083..4323917c1f62e7 100644
--- a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
@@ -496,6 +496,11 @@ void ReductionProcessor::addDeclareReduction(
     llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
         *reductionSymbols) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+
+  if (std::get<std::optional<omp::clause::Reduction::ReductionModifier>>(
+          reduction.t))
+    TODO(currentLocation, "Reduction modifiers are not supported");
+
   mlir::omp::DeclareReductionOp decl;
   const auto &redOperatorList{
       std::get<omp::clause::Reduction::ReductionIdentifiers>(reduction.t)};
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index bba1be27158ce7..eae4784169146e 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -136,6 +136,11 @@ TYPE_PARSER(construct<OmpReductionOperator>(Parser<DefinedOperator>{}) ||
     construct<OmpReductionOperator>(Parser<ProcedureDesignator>{}))
 
 TYPE_PARSER(construct<OmpReductionClause>(
+    maybe(
+        ("INSCAN" >> pure(OmpReductionClause::ReductionModifier::Inscan) ||
+            "TASK" >> pure(OmpReductionClause::ReductionModifier::Task) ||
+            "DEFAULT" >> pure(OmpReductionClause::ReductionModifier::Default)) /
+        ","),
     Parser<OmpReductionOperator>{} / ":", Parser<OmpObjectList>{}))
 
 // OMP 5.0 2.19.5.6 IN_REDUCTION (reduction-identifier: variable-name-list)
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index c06458833f0729..3398b395f198f8 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2090,6 +2090,8 @@ class UnparseVisitor {
     Walk(":", x.step);
   }
   void Unparse(const OmpReductionClause &x) {
+    Walk(std::get<std::optional<OmpReductionClause::ReductionModifier>>(x.t),
+        ",");
     Walk(std::get<OmpReductionOperator>(x.t));
     Put(":");
     Walk(std::get<OmpObjectList>(x.t));
@@ -2727,6 +2729,8 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
   WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
   WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
+  WALK_NESTED_ENUM(
+      OmpReductionClause, ReductionModifier) // OMP reduction-modifier
   WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
   WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
   WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index bf4debee1df34c..e85d8d1f7ab533 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2289,7 +2289,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
 bool OmpStructureChecker::CheckReductionOperators(
     const parser::OmpClause::Reduction &x) {
 
-  const auto &definedOp{std::get<0>(x.v.t)};
+  const auto &definedOp{std::get<parser::OmpReductionOperator>(x.v.t)};
   bool ok = false;
   common::visit(
       common::visitors{
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90 b/flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90
new file mode 100644
index 00000000000000..5e566466492ceb
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90
@@ -0,0 +1,13 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Reduction modifiers are not supported
+
+subroutine foo()
+  integer :: i, j
+  j = 0
+  !$omp do reduction (inscan, *: j)
+  do i = 1, 10
+    j = j + 1
+  end do
+end subroutine
diff --git a/flang/test/Parser/OpenMP/reduction-modifier.f90 b/flang/test/Parser/OpenMP/reduction-modifier.f90
new file mode 100644
index 00000000000000..d46aa709595925
--- /dev/null
+++ b/flang/test/Parser/OpenMP/reduction-modifier.f90
@@ -0,0 +1,20 @@
+! RUN: %flang_fc1 -fdebug-unparse-no-sema -fopenmp %s | FileCheck --ignore-case %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree-no-sema -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+subroutine foo()
+  integer :: i, j
+  j = 0
+! CHECK: !$OMP DO  REDUCTION(TASK,*:j)
+! PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
+! PARSE-TREE: | | | OmpBeginLoopDirective
+! PARSE-TREE: | | | | OmpLoopDirective -> llvm::omp::Directive = do
+! PARSE-TREE: | | | | OmpClauseList -> OmpClause -> Reduction -> OmpReductionClause
+! PARSE-TREE: | | | | | ReductionModifier = Task
+! PARSE-TREE: | | | | | OmpReductionOperator -> DefinedOperator -> IntrinsicOperator = Multiply
+! PARSE-TREE: | | | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j
+  !$omp do reduction (task, *: j)
+  do i = 1, 10
+    j = j + 1
+  end do
+  !$omp end do
+end
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index 6ce972adcf0fb0..daef02bcfc9a3f 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -949,7 +949,7 @@ struct ReductionT {
   using ReductionIdentifiers = ListT<type::ReductionIdentifierT<I, E>>;
   ENUM(ReductionModifier, Default, Inscan, Task);
   using TupleTrait = std::true_type;
-  std::tuple<ReductionIdentifiers, OPT(ReductionModifier), List> t;
+  std::tuple<OPT(ReductionModifier), ReductionIdentifiers, List> t;
 };
 
 // V5.2: [15.8.1] `memory-order` clauses

tblah

LGTM thanks for this!

tblah · 2024-04-11T09:53:17Z

flang/lib/Lower/OpenMP/Clauses.cpp

-  auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+  using wrapped = parser::OmpReductionClause;
+
+  CLAUSET_ENUM_CONVERT( //


nit

Suggested change

CLAUSET_ENUM_CONVERT( //

CLAUSET_ENUM_CONVERT(

Clang-format mutilates the layout without the trailing //.

kparzysz

LGTM. Thanks!

kparzysz · 2024-04-12T14:43:18Z

flang/lib/Lower/OpenMP/Clauses.cpp

-  auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+  using wrapped = parser::OmpReductionClause;
+
+  CLAUSET_ENUM_CONVERT( //


Clang-format mutilates the layout without the trailing //.

Accept the reduction modifier in the Flang parser. Issue a TODO message during lowering. OpenMP 5.0 introduced the reduction modifier. Details can be seen in 2.19.5.4 reductionClause. OpenMP 5.2 relevant section is 5.5.8reductionClause. This will help remove some of the parser errors highlighted in the following post and also bring it to a well defined behaviour (produce TODO errors for unsupported features, do not crash). https://discourse.llvm.org/t/proposal-rename-flang-new-to-flang/69462/60

kiranchandramohan force-pushed the red-modifier branch from 9cb2b4f to 8df51ec Compare April 6, 2024 12:52

[Flang][OpenMP] Accept the reduction modifier

c3103cd

kiranchandramohan force-pushed the red-modifier branch from 8df51ec to c3103cd Compare April 6, 2024 15:23

Merge branch 'main' into red-modifier

8b0b057

kiranchandramohan marked this pull request as ready for review April 8, 2024 10:37

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 Apr 8, 2024

kiranchandramohan requested review from DavidTruby, tblah, raghavendhra and kparzysz April 8, 2024 10:37

tblah approved these changes Apr 11, 2024

View reviewed changes

kparzysz approved these changes Apr 12, 2024

View reviewed changes

kiranchandramohan merged commit 17cb8a5 into llvm:main Apr 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Flang][OpenMP] Accept the reduction modifier #86492

[Flang][OpenMP] Accept the reduction modifier #86492

Uh oh!

kiranchandramohan commented Mar 25, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 25, 2024

Uh oh!

github-actions bot commented Mar 25, 2024 •

edited

Loading

Uh oh!

llvmbot commented Apr 8, 2024 •

edited

Loading

Uh oh!

tblah left a comment

Uh oh!

tblah Apr 11, 2024

Uh oh!

kparzysz Apr 12, 2024

Uh oh!

kparzysz left a comment

Uh oh!

kparzysz Apr 12, 2024

Uh oh!

Uh oh!

[Flang][OpenMP] Accept the reduction modifier #86492

[Flang][OpenMP] Accept the reduction modifier #86492

Uh oh!

Conversation

kiranchandramohan commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 25, 2024

Uh oh!

github-actions bot commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

tblah Apr 11, 2024

Choose a reason for hiding this comment

Uh oh!

kparzysz Apr 12, 2024

Choose a reason for hiding this comment

Uh oh!

kparzysz left a comment

Choose a reason for hiding this comment

Uh oh!

kparzysz Apr 12, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kiranchandramohan commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

llvmbot commented Apr 8, 2024 •

edited

Loading