[flang][OpenMP] Generate implicit mapping for reduction variable. (#15)

* Generate implicit mapping for reduction variable.

The result of the reduction was not being copied back to reduction variable on the host. To sidestep this problem, an explicit map(tofrom: var) has to be added in the code to get the result back.

This patch tries to solve this problem by generating an implicit tofrom mapping for reduction variable. All variables used in the target region (and without an explicit mapping) gets an implcit mapping in getTargetOP. We switch on TO,FROM bits for reduction varibles on this mapping.

* Move code to get the reduction variables into separate function.

Author: abidh

flang/lib/Lower/OpenMP.cpp

@@ -643,6 +643,9 @@ class ClauseProcessor {
                   llvm::SmallVectorImpl<mlir::Location> *mapSymLocs = nullptr,
                   llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
                       *mapSymbols = nullptr) const;
+  bool processTargetReduction(
+      llvm::SmallVector<const Fortran::semantics::Symbol *> &reductionSymbols)
+      const;
   bool processReduction(
       mlir::Location currentLocation,
       llvm::SmallVectorImpl<mlir::Value> &reductionVars,
@@ -1095,6 +1098,21 @@ class ReductionProcessor {
     return decl;
   }
 
+  static void addReductionSym(
+      const Fortran::parser::OmpReductionClause &reduction,
+      llvm::SmallVector<const Fortran::semantics::Symbol *> &Symbols) {
+    const auto &objectList{
+        std::get<Fortran::parser::OmpObjectList>(reduction.t)};
+
+    for (const Fortran::parser::OmpObject &ompObject : objectList.v) {
+      if (const auto *name{
+              Fortran::parser::Unwrap<Fortran::parser::Name>(ompObject)}) {
+        if (const Fortran::semantics::Symbol * symbol{name->symbol})
+          Symbols.push_back(symbol);
+      }
+    }
+  }
+
   /// Creates a reduction declaration and associates it with an OpenMP block
   /// directive.
   static void addReductionDecl(
@@ -2011,6 +2029,17 @@ bool ClauseProcessor::processMap(
       });
 }
 
+bool ClauseProcessor::processTargetReduction(
+    llvm::SmallVector<const Fortran::semantics::Symbol *> &reductionSymbols)
+    const {
+  return findRepeatableClause<ClauseTy::Reduction>(
+      [&](const ClauseTy::Reduction *reductionClause,
+          const Fortran::parser::CharBlock &) {
+        ReductionProcessor rp;
+        rp.addReductionSym(reductionClause->v, reductionSymbols);
+      });
+}
+
 bool ClauseProcessor::processReduction(
     mlir::Location currentLocation,
     llvm::SmallVectorImpl<mlir::Value> &reductionVars,
@@ -3153,6 +3182,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
   llvm::SmallVector<mlir::Type> mapSymTypes;
   llvm::SmallVector<mlir::Location> mapSymLocs;
   llvm::SmallVector<const Fortran::semantics::Symbol *> mapSymbols;
+  llvm::SmallVector<const Fortran::semantics::Symbol *> reductionSymbols;
 
   ClauseProcessor cp(converter, clauseList);
   cp.processDevice(stmtCtx, deviceOperand);
@@ -3178,6 +3208,9 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
            .getIsTargetDevice())
     cp.processNowait(nowaitAttr);
 
+  if (outerCombined)
+    cp.processTargetReduction(reductionSymbols);
+
   // 5.8.1 Implicit Data-Mapping Attribute Rules
   // The following code follows the implicit data-mapping rules to map all the
   // symbols used inside the region that have not been explicitly mapped using
@@ -3222,7 +3255,11 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
         if (auto refType = baseOp.getType().dyn_cast<fir::ReferenceType>())
           eleType = refType.getElementType();
 
-        if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) {
+        // Do a tofrom map for reduction variables.
+        if (llvm::find(reductionSymbols, &sym) != reductionSymbols.end()) {
+          mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+          mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
+        } else if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) {
           captureKind = mlir::omp::VariableCaptureKind::ByCopy;
         } else if (!fir::isa_builtin_cptr_type(eleType)) {
           mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;

flang/test/Lower/OpenMP/reduction_var_map.f90

@@ -0,0 +1,43 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that if reduction clause is on a combined target
+! construct, there is an implicit map(tofrom) for each reduction variable.
+
+! construct with target
+subroutine omp_target_combined
+   implicit none
+   integer(kind = 8) :: s1
+   integer(kind = 8) :: s2
+   integer(kind = 4) ::  i
+   s1 = 1
+   s2 = 1
+   !$omp target teams distribute parallel do reduction(+:s1) reduction(+:s2)
+      do i=1,1000
+          s1 = s1 + i
+          s2 = s2 + i
+      end do
+   !$omp end target teams distribute parallel do
+   return
+end subroutine omp_target_combined
+!CHECK-LABEL: func.func @_QPomp_target_combined() {
+!CHECK: omp.map_info var_ptr({{.*}} : !fir.ref<i64>, i64) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref<i64> {name = "s1"}
+!CHECK: omp.map_info var_ptr({{.*}} : !fir.ref<i64>, i64) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref<i64> {name = "s2"}
+!CHECK: omp.map_info var_ptr({{.*}} : !fir.ref<i32>, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i32> {name = "i"}
+
+subroutine omp_target_team_separate
+   implicit none
+   integer(kind = 8) :: s3
+   integer i
+   s3 = 1
+   !$omp target
+   s3 = 2
+   !$omp teams distribute parallel do reduction(+:s3)
+      do i=1,1000
+         s3 = s3 + i
+      end do
+   !$omp end teams distribute parallel do
+   !$omp end target
+   return
+end subroutine omp_target_team_separate
+!CHECK-LABEL: func.func @_QPomp_target_team_separate() {
+!CHECK:  omp.map_info var_ptr({{.*}} : !fir.ref<i64>, i64) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i64> {name = "s3"}