[Flang][openmp][5.0] Add task_reduction clause.

[sameeranjoshi]

Summary

See OMP-5.0 2.19.5.5 task_reduction Clause.
To add a positive test case we need taskgroup directive which is not added hence skipping the test.
This is a dependency for taskgroup construct.

Co-authored-by: Valentin Clement <[e-mail]>

[clementval]

flang/include/flang/Parser/parse-tree.h

3427

Is there a problem with the formatting here?

3430

Is there a good reason why OmpReductionClause takes a list of Designator and here you take an OmpObjectList?

flang/lib/Parser/unparse.cpp

2020

Extra blank line.

[sameeranjoshi]

flang/include/flang/Parser/parse-tree.h

3430

I did ask once the same in slack channel.
But I think the complicated checks can be done in semantic part given the tablegen framework.
https://flang-compiler.slack.com/archives/CGB8G85QV/p1603382608040500

[clementval]

flang/include/flang/Parser/parse-tree.h

3430

I think they should be the same and possibly use the same node for that.

[sameeranjoshi]

Update OmpReductionClause to use OmpObjectList.
Make OmpTaskReductionClause depend on OmpReductionClause.
This change was needed to create consistency among the reduction clauses.

[sameeranjoshi]

Thank you for reviewing.

flang/include/flang/Parser/parse-tree.h

3430

OmpReductionClause now uses OmpObjectList.

[clementval]

flang/lib/Parser/openmp-parsers.cpp

108

Do you really need this indirection? Why not using OmpReductionClause directly?

[clementval]

Remove indirection and use OmpReductionClause directly.

[clementval]

flang/lib/Parser/openmp-parsers.cpp

226

@sameeranjoshi After our slack discussion, I hope you don't mind but I updated your patch with a working version without the indirection.

The reduction clause is actually not done "right" at the moment since it doesn't follow other clauses. It should be done in a similar way as TASK_REDUCTION now. I guess we can do this in a separated patch.

flang/lib/Parser/unparse.cpp

2007

This will be changed in a follow up patch to reflect the change in TaskReduction and Reduction.

[clementval]

I guess it looks like what we have discussed now.

If you want to attribute correctly the co-authorship of this patch, you can add Co-authored-by: Valentin Clement <[e-mail]> at the end of the commit message. GitHub will do the rest.

[kiranchandramohan]

@yhegde Will the changes in this patch affect your reduction patch (https://reviews.llvm.org/D90697)?

[clementval]

flang/lib/Parser/openmp-parsers.cpp

226

update to reduction clause are done in D93482

[yhegde]

In D93105#2461225, @kiranchandramohan wrote:

@yhegde Will the changes in this patch affect your reduction patch (https://reviews.llvm.org/D90697)?

Will take a look and get back to you.

[yhegde]

In D93105#2461225, @kiranchandramohan wrote:

@yhegde Will the changes in this patch affect your reduction patch (https://reviews.llvm.org/D90697)?

Yes. Rt now I am directly getting the list of Designators . Now it will become a level of indirection with the list of OmpObjects.
I may be missing something but OpenMP 4.5 and 5.0 Reduction clause restrictions looks very similar , why this one level of indirection required?! Is it because functions names in Reduction clause can come from common blocks ?!

[sameeranjoshi]

In D93105#2464268, @yhegde wrote:

In D93105#2461225, @kiranchandramohan wrote:

@yhegde Will the changes in this patch affect your reduction patch (https://reviews.llvm.org/D90697)?

Yes. Rt now I am directly getting the list of Designators . Now it will become a level of indirection with the list of OmpObjects.
I may be missing something but OpenMP 4.5 and 5.0 Reduction clause restrictions looks very similar , why this one level of indirection required?! Is it because functions names in Reduction clause can come from common blocks ?!

I am not sure if I completely understand your question.
struct OmpObject wraps over std::variant<Designator, /*common block*/ Name> u, and if you were trying to ask why did we change that from Designator to OmpObject check comments at https://reviews.llvm.org/D93105#inline-870822.

[clementval]

The co-authorship must be in the commit message. Updating phab doesn't change what was landed. Look at https://github.com/llvm/llvm-project/commit/8a58f21f5b6c228137a9b87906fe5b720c4d1dfb, the co-authorship is correctly displayed on GitHub. This is not the case with the commit you landed for this revision.

[clementval]

Reopen the revision since it was reverted and other patches depend on it.

flang/include/flang/Parser/dump-parse-tree.h

return llvm::Twine(
         .str();
   }
   NODE(parser, OmpNowait)
   NODE(parser, OmpObject)
   NODE(parser, OmpObjectList)
   NODE(parser, OmpProcBindClause)
   NODE_ENUM(OmpProcBindClause, Type)
   NODE(parser, OmpReductionClause)
+  NODE(parser, OmpTaskReductionClause)
   NODE(parser, OmpReductionCombiner)
   NODE(OmpReductionCombiner, FunctionCombiner)
   NODE(parser, OmpReductionInitializerClause)
   NODE(parser, OmpReductionOperator)
   NODE(parser, OmpAllocateClause)
   NODE(OmpAllocateClause, Allocator)
   NODE(parser, OmpScheduleClause)
   NODE_ENUM(OmpScheduleClause, ScheduleType)

flang/include/flang/Parser/parse-tree.h

struct OmpReductionOperator {
   UNION_CLASS_BOILERPLATE(OmpReductionOperator);
   std::variant<DefinedOperator, ProcedureDesignator> u;
 };
 
 // 2.15.3.6 reduction-clause -> REDUCTION (reduction-identifier:
 //                                         variable-name-list)
 struct OmpReductionClause {
   TUPLE_CLASS_BOILERPLATE(OmpReductionClause);
-  std::tuple<OmpReductionOperator, std::list<Designator>> t;
+  std::tuple<OmpReductionOperator, OmpObjectList> t;
 };
 
+// [OMP-5.0] 2.19.5.5 task-reduction-clause ->
+//                    TASK_REDUCTION (reduction-identifier: variable-name-list)
+WRAPPER_CLASS(OmpTaskReductionClause, OmpReductionClause);
+
 // OMP 5.0 2.11.4 allocate-clause -> ALLOCATE ([allocator:] variable-name-list)
 struct OmpAllocateClause {
   TUPLE_CLASS_BOILERPLATE(OmpAllocateClause);
   WRAPPER_CLASS(Allocator, ScalarIntExpr);
   std::tuple<std::optional<Allocator>, OmpObjectList> t;
 };
 
 // 2.13.9 depend-vec-length -> +/- non-negative-constant
 struct OmpDependSinkVecLength {
   TUPLE_CLASS_BOILERPLATE(OmpDependSinkVecLength);

flang/lib/Parser/openmp-parsers.cpp

maybe(
         ":"),
     scalarLogicalExpr))
 
 // 2.15.3.6 REDUCTION (reduction-identifier: variable-name-list)
 TYPE_PARSER(construct<OmpReductionOperator>(Parser<DefinedOperator>{}) ||
     construct<OmpReductionOperator>(Parser<ProcedureDesignator>{}))
 
 TYPE_PARSER(construct<OmpReductionClause>(
-    Parser<OmpReductionOperator>{} / ":", nonemptyList(designator)))
+    Parser<OmpReductionOperator>{} / ":", Parser<OmpObjectList>{}))
+
+//[OMP-5.0] 2.19.5.5 TASK_REDUCTION (reduction-identifier: variable-name-list)
+TYPE_PARSER(construct<OmpTaskReductionClause>(Parser<OmpReductionClause>{}))
 
 // OMP 5.0 2.11.4  ALLOCATE ([allocator:] variable-name-list)
 TYPE_PARSER(construct<OmpAllocateClause>(
     maybe(construct<OmpAllocateClause::Allocator>(scalarIntExpr) / ":"),
     Parser<OmpObjectList>{}))
 
 // 2.13.9 DEPEND (SOURCE | SINK : vec | (IN | OUT | INOUT) : list
 TYPE_PARSER(construct<OmpDependSinkVecLength>(
TYPE_PARSER(
     "PRIORITY" >> construct<OmpClause>(construct<OmpClause::Priority>(
                       parenthesized(scalarIntExpr))) ||
     "PRIVATE" >> construct<OmpClause>(construct<OmpClause::Private>(
                      parenthesized(Parser<OmpObjectList>{}))) ||
     "PROC_BIND" >>
         construct<OmpClause>(parenthesized(Parser<OmpProcBindClause>{})) ||
     "REDUCTION" >>
         construct<OmpClause>(parenthesized(Parser<OmpReductionClause>{})) ||
+    "TASK_REDUCTION" >>
+        construct<OmpClause>(parenthesized(Parser<OmpTaskReductionClause>{})) ||
     "RELAXED" >> construct<OmpClause>(construct<OmpClause::Relaxed>()) ||
     "RELEASE" >> construct<OmpClause>(construct<OmpClause::Release>()) ||
     "SAFELEN" >> construct<OmpClause>(construct<OmpClause::Safelen>(
                      parenthesized(scalarIntConstantExpr))) ||
     "SCHEDULE" >>
         construct<OmpClause>(parenthesized(Parser<OmpScheduleClause>{})) ||
     "SEQ_CST" >> construct<OmpClause>(construct<OmpClause::SeqCst>()) ||
     "SHARED" >> construct<OmpClause>(construct<OmpClause::Shared>(
                     parenthesized(Parser<OmpObjectList>{}))) ||

flang/lib/Parser/unparse.cpp

void Unparse(const OmpLinearClause::WithoutModifier &x) {
     Put(")");
   }
   void Unparse(const OmpLinearClause::WithModifier &x) {
     Word("LINEAR("), Walk(x.modifier), Put("("), Walk(x.names, ","), Put(")");
     Walk(":", x.step);
     Put(")");
   }
   void Unparse(const OmpReductionClause &x) {
     Word("REDUCTION(");
     Walk(std::get<OmpReductionOperator>(x.t));
     Put(":");
-    Walk(std::get<std::list<Designator>>(x.t), ",");
+    Walk(std::get<OmpObjectList>(x.t));
     Put(")");
   }
+  void Unparse(const OmpTaskReductionClause &x) {
+    Word("TASK_REDUCTION(");
+    Walk(x.v);
+  }
   void Unparse(const OmpAllocateClause &x) {
     Word("ALLOCATE(");
     Walk(std::get<std::optional<OmpAllocateClause::Allocator>>(x.t), ":");
     Walk(std::get<OmpObjectList>(x.t));
     Put(")");
   }
   void Unparse(const OmpDependSinkVecLength &x) {
     Walk(std::get<DefinedOperator>(x.t));
     Walk(std::get<ScalarIntConstantExpr>(x.t));
   }
   void Unparse(const OmpDependSinkVec &x) {
     Walk(std::get<Name>(x.t));

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   void Enter(const parser::OmpDefaultmapClause &);
   void Enter(const parser::OmpDependClause &);
   void Enter(const parser::OmpDistScheduleClause &);
   void Enter(const parser::OmpIfClause &);
   void Enter(const parser::OmpLinearClause &);
   void Enter(const parser::OmpMapClause &);
   void Enter(const parser::OmpProcBindClause &);
   void Enter(const parser::OmpReductionClause &);
+  void Enter(const parser::OmpTaskReductionClause &);
   void Enter(const parser::OmpScheduleClause &);
 
 private:
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
 
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,

flang/lib/Semantics/check-omp-structure.cpp

 
 // Following clauses have a seperate node in parse-tree.h.
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAllocateClause, OMPC_allocate)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDefaultClause, OMPC_default)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDistScheduleClause, OMPC_dist_schedule)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpNowait, OMPC_nowait)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpProcBindClause, OMPC_proc_bind)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpReductionClause, OMPC_reduction)
+CHECK_SIMPLE_PARSER_CLAUSE(OmpTaskReductionClause, OMPC_task_reduction)
 
 // Restrictions specific to each clause are implemented apart from the
 // generalized restrictions.
 void OmpStructureChecker::Enter(const parser::OmpAlignedClause &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_aligned);
 
   if (const auto &expr{
           std::get<std::optional<parser::ScalarIntConstantExpr>>(x.t)}) {

flang/test/Semantics/omp-clause-validity01.f90

 !                      linear-clause |
 !                      aligned-clause |
 !                      private-clause |
 !                      lastprivate-clause |
 !                      reduction-clause |
 !                      collapse-clause
 
   a = 0.0
-  !$omp simd private(b) reduction(+:a)
+  !ERROR: TASK_REDUCTION clause is not allowed on the SIMD directive
+  !$omp simd private(b) reduction(+:a) task_reduction(+:a)
   do i = 1, N
      a = a + b + 3.14
   enddo
 
   !ERROR: At most one SAFELEN clause can appear on the SIMD directive
   !$omp simd safelen(1) safelen(2)
   do i = 1, N
      a = 3.14
!                          nogroup-clause
 
   !ERROR: GRAINSIZE and NUM_TASKS clauses are mutually exclusive and may not appear on the same TASKLOOP directive
   !$omp taskloop num_tasks(3) grainsize(2)
   do i = 1,N
      a = 3.14
   enddo
 
   !ERROR: At most one NUM_TASKS clause can appear on the TASKLOOP directive
-  !$omp taskloop num_tasks(3) num_tasks(2)
+  !ERROR: TASK_REDUCTION clause is not allowed on the TASKLOOP directive
+  !$omp taskloop num_tasks(3) num_tasks(2) task_reduction(*:a)
   do i = 1,N
     a = 3.14
   enddo
 
 ! 2.13.1 master
 
   !$omp parallel
   !$omp master

llvm/include/llvm/Frontend/OpenMP/OMP.td

 }
 def OMPC_IsDevicePtr : Clause<"is_device_ptr"> {
   let clangClass = "OMPIsDevicePtrClause";
   let flangClassValue = "Name";
   let isValueList = true;
 }
 def OMPC_TaskReduction : Clause<"task_reduction"> {
   let clangClass = "OMPTaskReductionClause";
+  let flangClass = "OmpTaskReductionClause";
 }
 def OMPC_InReduction : Clause<"in_reduction"> {
   let clangClass = "OMPInReductionClause";
 }
 def OMPC_UnifiedAddress : Clause<"unified_address"> {
   let clangClass = "OMPUnifiedAddressClause";
 }
 def OMPC_UnifiedSharedMemory : Clause<"unified_shared_memory"> {