[flang][openacc] Fix reduction operator parsing/unparsing

[clementval]

Summary

Update the way reduction operator are defined for the OpenACC parser.

[kiranchandramohan]

Sorry, I have not gone into detail here. But could you provide some motivation/explanation for this change? Is this because OpenACC accepts fewer or different reduction operators. Or is a similar change required in OpenMP too.

[clementval]

In D86296#2228691, @kiranchandramohan wrote:

Sorry, I have not gone into detail here. But could you provide some motivation/explanation for this change? Is this because OpenACC accepts fewer or different reduction operators. Or is a similar change required in OpenMP too.

In the current state, only reduction with + and * would be parsed and resolved correctly. We would need an intrinsic function name resolution in place to accept the others operators and also a semantic check to discard all intrinsic that are not accepted as reduction operators. So in order to simply thing, a simple enumeration seemed a better replacement. OpenACC doesn't accept user-defined reduction operator (as of 3.0) so we can make this simplification. For OpenMP, as you can use user-defined operator I guess you will have to put in place a function name resolution for those reduction operators. But in the current state the openmp reduction operator such as max or min are likely to fail at name resolution.

[kiranchandramohan]

LGTM. But because this is different from OpenMP handling it will be best if @klausler or @sscalpone approves.

Would you have issues matching the operation in Fortran and the reduction operation in the OpenACC directive since you are using your own custom enums?

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

3846

For my information: Why is source needed here?

[clementval]

In D86296#2230456, @kiranchandramohan wrote:

Would you have issues matching the operation in Fortran and the reduction operation in the OpenACC directive since you are using your own custom enums?

I'm not sure I get what you want to say here.

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

3846

Just to hold the source location information for error reporting. So error can be reported on the reduction operator.

[kiranchandramohan]

In D86296#2230534, @clementval wrote:

In D86296#2230456, @kiranchandramohan wrote:

Would you have issues matching the operation in Fortran and the reduction operation in the OpenACC directive since you are using your own custom enums?

I'm not sure I get what you want to say here.

What I meant to say here is that previously the OpenACCReductionOperator was defined in terms of struct DefinedOperator which in turn is defined in terms of DefinedOpName or IntrinsicOperator. If these are also used for the Fortran expressions in the parse tree then it is easy to match the OpenACC Reduction Operation and the operation in Fortran source where reduction is implied. If you use your own custom definitions for operations then there is some code that you have to write to do the matching.

[clementval]

In D86296#2230566, @kiranchandramohan wrote:

In D86296#2230534, @clementval wrote:

In D86296#2230456, @kiranchandramohan wrote:

Would you have issues matching the operation in Fortran and the reduction operation in the OpenACC directive since you are using your own custom enums?

I'm not sure I get what you want to say here.

What I meant to say here is that previously the OpenACCReductionOperator was defined in terms of struct DefinedOperator which in turn is defined in terms of DefinedOpName or IntrinsicOperator. If these are also used for the Fortran expressions in the parse tree then it is easy to match the OpenACC Reduction Operation and the operation in Fortran source where reduction is implied. If you use your own custom definitions for operations then there is some code that you have to write to do the matching.

Ok I see now. Yeah you are right in case I need to do a matching (which is still unclear at this stage) there would be some code to be written to do that. But my guess right now is that we can lower this directly to the dialect without the need to match an operation since what we need are the values taking part in the reduction to issue the correct operation.

[clementval]

Rebase

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

#include "llvm/Frontend/OpenACC/ACC.cpp.inc"
   NODE(parser, AccEndBlockDirective)
   NODE(parser, AccEndCombinedDirective)
   NODE(parser, AccGangArgument)
   NODE(parser, AccObject)
   NODE(parser, AccObjectList)
   NODE(parser, AccObjectListWithModifier)
   NODE(parser, AccObjectListWithReduction)
   NODE(parser, AccReductionOperator)
+  NODE_ENUM(parser::AccReductionOperator, Operator)
   NODE(parser, AccSizeExpr)
   NODE(parser, AccSizeExprList)
   NODE(parser, AccStandaloneDirective)
   NODE(parser, AccLoopDirective)
   NODE(parser, AccWaitArgument)
   static std::string GetNodeName(const llvm::acc::Directive &x) {
     return llvm::Twine(
         "llvm::acc::Directive = ", llvm::acc::getOpenACCDirectiveName(x))

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

 
 struct AccObjectListWithModifier {
   TUPLE_CLASS_BOILERPLATE(AccObjectListWithModifier);
   std::tuple<std::optional<AccDataModifier>, AccObjectList> t;
 };
 
 // 2.5.13: + | * | max | min | iand | ior | ieor | .and. | .or. | .eqv. | .neqv.
 struct AccReductionOperator {
-  UNION_CLASS_BOILERPLATE(AccReductionOperator);
-  std::variant<DefinedOperator, ProcedureDesignator> u;
+  ENUM_CLASS(
+      Operator, Plus, Multiply, Max, Min, Iand, Ior, Ieor, And, Or, Eqv, Neqv)
+  WRAPPER_CLASS_BOILERPLATE(AccReductionOperator, Operator);
+  CharBlock source;
 };
 
 struct AccObjectListWithReduction {
   TUPLE_CLASS_BOILERPLATE(AccObjectListWithReduction);
   std::tuple<AccReductionOperator, AccObjectList> t;
 };
 
 struct AccWaitArgument {

flang/lib/Parser/openacc-parsers.cpp

 //   [num:]int-expr
 //   static:size-expr
 TYPE_PARSER(construct<AccGangArgument>(maybe(scalarIntExpr),
                 maybe(","_tok / "STATIC:" >> Parser<AccSizeExpr>{})) ||
     construct<AccGangArgument>(maybe("NUM:" >> scalarIntExpr),
         maybe(","_tok / "STATIC:" >> Parser<AccSizeExpr>{})))
 
 // 2.5.13 Reduction
-TYPE_PARSER(construct<AccReductionOperator>(Parser<DefinedOperator>{}) ||
-    construct<AccReductionOperator>(Parser<ProcedureDesignator>{}))
+// Operator for reduction
+TYPE_PARSER(sourced(construct<AccReductionOperator>(
+    first("+" >> pure(AccReductionOperator::Operator::Plus),
+        "*" >> pure(AccReductionOperator::Operator::Multiply),
+        "MAX" >> pure(AccReductionOperator::Operator::Max),
+        "MIN" >> pure(AccReductionOperator::Operator::Min),
+        "IAND" >> pure(AccReductionOperator::Operator::Iand),
+        "IOR" >> pure(AccReductionOperator::Operator::Ior),
+        "IEOR" >> pure(AccReductionOperator::Operator::Ieor),
+        ".AND." >> pure(AccReductionOperator::Operator::And),
+        ".OR." >> pure(AccReductionOperator::Operator::Or),
+        ".EQV." >> pure(AccReductionOperator::Operator::Eqv),
+        ".NEQV." >> pure(AccReductionOperator::Operator::Neqv)))))
 
 // 2.5.14 Default clause
 TYPE_PARSER(construct<AccDefaultClause>(
     parenthesized(first("NONE" >> pure(AccDefaultClause::Arg::None),
         "PRESENT" >> pure(AccDefaultClause::Arg::Present)))))
 
 // Modifier for copyin, copyout, cache and create
 TYPE_PARSER(construct<AccDataModifier>(

flang/lib/Parser/unparse.cpp

#include "llvm/Frontend/OpenACC/ACC.cpp.inc"
   void Unparse(const AccObject &x) {
     std::visit(common::visitors{
                    [&](const Designator &y) { Walk(y); },
                    [&](const Name &y) { Put("/"), Walk(y), Put("/"); },
                },
         x.u);
   }
   void Unparse(const AccObjectList &x) { Walk(x.v, ","); }
+  void Unparse(const AccReductionOperator::Operator &x) {
+    Word(AccReductionOperator::EnumToString(x));
+  }
   void Unparse(const AccObjectListWithReduction &x) {
     Walk(std::get<AccReductionOperator>(x.t));
     Put(":");
     Walk(std::get<AccObjectList>(x.t));
   }
   void Unparse(const OpenACCCacheConstruct &x) {
     BeginOpenACC();
     Word("!$ACC ");

flang/test/Semantics/acc-clause-validity.f90

 !   2.11 Parallel Loop
 !   2.11 Kernels Loop
 !   2.11 Serial Loop
 
 program openacc_clause_validity
 
   implicit none
 
-  integer :: i, j
-  integer :: N = 256
+  integer :: i, j, b
+  integer, parameter :: N = 256
+  integer, dimension(N) :: c
+  logical, dimension(N) :: d, e
+  real :: reduction_r
+  logical :: reduction_l
+
   !ERROR: At least one clause is required on the DECLARE directive
   !$acc declare
   real(8) :: a(256)
 
   !ERROR: At least one of ATTACH, COPYIN, CREATE clause must appear on the ENTER DATA directive
   !$acc enter data
 
   !ERROR: Only the READONLY modifier is allowed for the COPYIN clause on the ENTER DATA directive
program openacc_clause_validity
 
   !$acc serial loop
   do i = 1, N
     a(i) = 3.14
   end do
   !ERROR: Unmatched END PARALLEL LOOP directive
   !$acc end parallel loop
 
+  !$acc parallel loop reduction(+: reduction_r)
+  do i = 1, N
+    reduction_r = a(i) + i
+  end do
+
+  !$acc parallel loop reduction(*: reduction_r)
+  do i = 1, N
+    reduction_r = reduction_r * (a(i) + i)
+  end do
+
+  !$acc parallel loop reduction(min: reduction_r)
+  do i = 1, N
+    reduction_r = min(reduction_r, a(i) * i)
+  end do
+
+  !$acc parallel loop reduction(max: reduction_r)
+  do i = 1, N
+    reduction_r = max(reduction_r, a(i) * i)
+  end do
+
+  !$acc parallel loop reduction(iand: b)
+  do i = 1, N
+    b = iand(b, c(i))
+  end do
+
+  !$acc parallel loop reduction(ior: b)
+  do i = 1, N
+    b = ior(b, c(i))
+  end do
+
+  !$acc parallel loop reduction(ieor: b)
+  do i = 1, N
+    b = ieor(b, c(i))
+  end do
+
+  !$acc parallel loop reduction(.and.: reduction_l)
+  do i = 1, N
+    reduction_l = d(i) .and. e(i)
+  end do
+
+  !$acc parallel loop reduction(.or.: reduction_l)
+  do i = 1, N
+    reduction_l = d(i) .or. e(i)
+  end do
+
+  !$acc parallel loop reduction(.eqv.: reduction_l)
+  do i = 1, N
+    reduction_l = d(i) .eqv. e(i)
+  end do
+
+  !$acc parallel loop reduction(.neqv.: reduction_l)
+  do i = 1, N
+    reduction_l = d(i) .neqv. e(i)
+  end do
+
   !$acc kernels wait(1, 2) async(3)
   !$acc end kernels
 
   !$acc kernels wait(queues: 1, 2) async(3)
   !$acc end kernels
 
   !$acc kernels wait(devnum: 1: 1, 2) async(3)
   !$acc end kernels