[flang][openmp] Make Reduction clause part of OmpClause

[clementval]

Summary

After discussion in D93105 we found that the reduction clause was not following
the common OmpClause convention. This patch makes reduction clause part of OmpClause
with a value of OmpReductionClause in a similar way than task_reduction.
The unparse function for OmpReductionClause is adapted since the keyword and parenthesis
are issued by the corresponding unparse function for parser::OmpClause::Reduction.

[sameeranjoshi]

+1 for finding the issue.
Thanks for the efforts and LGMT.

[sameeranjoshi]

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

498–499

A quick question on this-
Why are they not modeled as OmpClause nodes any reason ?
As I see all of them seem to have definition inside OMP.td file.

[clementval]

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

498–499

Any clause will have a definition in OMP.td. If they have a flangClass defined it will remove them from OmpClause.

I guess we can have a look at all of them and see if there is still a good reason why they are not part of OmpClause. Might be a technical-debt from the refactoring when we switch to TableGen. In OpenACC there is no such clause.

Let me know if you want to have a look at this. Otherwise I can do it.

[sameeranjoshi]

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

498–499

Let's start with OMPC_Allocate clause.
And the reference patch is D93482
What does technical-debt mean ?

[clementval]

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

498–499

Just mean that the initial approach choose a simple path instead of a generic enough one. So when we refactored it we kept some of it and now it's good time to move everything to the same concept.

[yhegde]

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

421

Can we get an Enter function (Enter(const parser::OmpClause::Reduction &x)) to check the restrictions on Reduction clause.

[clementval]

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

421

Can be added when necessary

[clementval]

Rebase

flang/lib/Parser/openmp-parsers.cpp

TYPE_PARSER(
     "ORDERED" >> construct<OmpClause>(construct<OmpClause::Ordered>(
                      maybe(parenthesized(scalarIntConstantExpr)))) ||
     "PRIORITY" >> construct<OmpClause>(construct<OmpClause::Priority>(
                       parenthesized(scalarIntExpr))) ||
     "PRIVATE" >> construct<OmpClause>(construct<OmpClause::Private>(
                      parenthesized(Parser<OmpObjectList>{}))) ||
     "PROC_BIND" >> construct<OmpClause>(construct<OmpClause::ProcBind>(
                        parenthesized(Parser<OmpProcBindClause>{}))) ||
-    "REDUCTION" >>
-        construct<OmpClause>(parenthesized(Parser<OmpReductionClause>{})) ||
+    "REDUCTION" >> construct<OmpClause>(construct<OmpClause::Reduction>(
+                       parenthesized(Parser<OmpReductionClause>{}))) ||
     "TASK_REDUCTION" >>
         construct<OmpClause>(construct<OmpClause::TaskReduction>(
             parenthesized(Parser<OmpReductionClause>{}))) ||
     "RELAXED" >> construct<OmpClause>(construct<OmpClause::Relaxed>()) ||
     "RELEASE" >> construct<OmpClause>(construct<OmpClause::Release>()) ||
     "SAFELEN" >> construct<OmpClause>(construct<OmpClause::Safelen>(
                      parenthesized(scalarIntConstantExpr))) ||
     "SCHEDULE" >>

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<OmpObjectList>(x.t));
-    Put(")");
   }
   void Unparse(const OmpAllocateClause &x) {
     Walk(std::get<std::optional<OmpAllocateClause::Allocator>>(x.t));
     Put(":");
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpDependSinkVecLength &x) {
     Walk(std::get<DefinedOperator>(x.t));

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

#include "llvm/Frontend/OpenMP/OMP.inc"
   void Enter(const parser::OmpClause::Lastprivate &);
   void Enter(const parser::OmpClause::NumTasks &);
   void Enter(const parser::OmpClause::NumTeams &);
   void Enter(const parser::OmpClause::NumThreads &);
   void Enter(const parser::OmpClause::Ordered &);
   void Enter(const parser::OmpClause::Priority &);
   void Enter(const parser::OmpClause::Private &);
   void Enter(const parser::OmpClause::ProcBind &);
+  void Enter(const parser::OmpClause::Reduction &);
   void Enter(const parser::OmpClause::Safelen &);
   void Enter(const parser::OmpClause::Shared &);
   void Enter(const parser::OmpClause::Simdlen &);
   void Enter(const parser::OmpClause::TaskReduction &);
   void Enter(const parser::OmpClause::ThreadLimit &);
   void Enter(const parser::OmpClause::To &);
   void Enter(const parser::OmpClause::Link &);
   void Enter(const parser::OmpClause::Uniform &);
#include "llvm/Frontend/OpenMP/OMP.inc"
   void Enter(const parser::OmpAtomicCapture &);
   void Leave(const parser::OmpAtomic &);
   void Enter(const parser::OmpAlignedClause &);
   void Enter(const parser::OmpDefaultmapClause &);
   void Enter(const parser::OmpDependClause &);
   void Enter(const parser::OmpIfClause &);
   void Enter(const parser::OmpLinearClause &);
   void Enter(const parser::OmpMapClause &);
-  void Enter(const parser::OmpReductionClause &);
   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

 CHECK_SIMPLE_CLAUSE(From, OMPC_from)
 CHECK_SIMPLE_CLAUSE(Inbranch, OMPC_inbranch)
 CHECK_SIMPLE_CLAUSE(IsDevicePtr, OMPC_is_device_ptr)
 CHECK_SIMPLE_CLAUSE(Lastprivate, OMPC_lastprivate)
 CHECK_SIMPLE_CLAUSE(Link, OMPC_link)
 CHECK_SIMPLE_CLAUSE(Mergeable, OMPC_mergeable)
 CHECK_SIMPLE_CLAUSE(Nogroup, OMPC_nogroup)
 CHECK_SIMPLE_CLAUSE(Notinbranch, OMPC_notinbranch)
 CHECK_SIMPLE_CLAUSE(Nowait, OMPC_nowait)
+CHECK_SIMPLE_CLAUSE(Reduction, OMPC_reduction)
 CHECK_SIMPLE_CLAUSE(TaskReduction, OMPC_task_reduction)
 CHECK_SIMPLE_CLAUSE(To, OMPC_to)
 CHECK_SIMPLE_CLAUSE(Uniform, OMPC_uniform)
 CHECK_SIMPLE_CLAUSE(Untied, OMPC_untied)
 CHECK_SIMPLE_CLAUSE(UseDevicePtr, OMPC_use_device_ptr)
 CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
 CHECK_SIMPLE_CLAUSE(Acquire, OMPC_acquire)
 CHECK_SIMPLE_CLAUSE(SeqCst, OMPC_seq_cst)
std::visit(
                 }
               }
             },
             [&](const parser::Name &name) {},
         },
         ompObject.u);
   }
 }
 // Following clauses have a seperate node in parse-tree.h.
-CHECK_SIMPLE_PARSER_CLAUSE(OmpReductionClause, OMPC_reduction)
 // Atomic-clause
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicRead, OMPC_read)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicWrite, OMPC_write)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicUpdate, OMPC_update)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicCapture, OMPC_capture)
 
 void OmpStructureChecker::Leave(const parser::OmpAtomicRead &) {
   CheckNotAllowedIfClause(llvm::omp::Clause::OMPC_read,
       {llvm::omp::Clause::OMPC_release, llvm::omp::Clause::OMPC_acq_rel});

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

def OMPC_LastPrivate : Clause<"lastprivate"> {
   let flangClassValue = "OmpObjectList";
 }
 def OMPC_Shared : Clause<"shared"> {
   let clangClass = "OMPSharedClause";
   let flangClassValue = "OmpObjectList";
 }
 def OMPC_Reduction : Clause<"reduction"> {
   let clangClass = "OMPReductionClause";
-  let flangClass = "OmpReductionClause";
+  let flangClassValue = "OmpReductionClause";
 }
 def OMPC_Linear : Clause<"linear"> {
   let clangClass = "OMPLinearClause";
   let flangClass = "OmpLinearClause";
 }
 def OMPC_Aligned : Clause<"aligned"> {
   let clangClass = "OMPAlignedClause";
   let flangClass = "OmpAlignedClause";