Skip to content

[flang][OpenMP] Add support for copyprivate #80485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

luporl
Copy link
Contributor

@luporl luporl commented Feb 2, 2024

Add initial handling of OpenMP copyprivate clause in Flang.

When lowering copyprivate, Flang generates the copy function
needed by each variable and builds the appropriate
omp.single's CopyPrivateVarList.

This is patch 3 of 4, to add support for COPYPRIVATE in Flang.
Original PR: #73128

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Feb 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 2, 2024

@llvm/pr-subscribers-flang-openmp

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

Author: Leandro Lupori (luporl)

Changes

Add initial handling of OpenMP copyprivate clause in Flang.

When lowering copyprivate, Flang generates the copy function
needed by each variable and builds the appropriate
omp.single's CopyPrivateVarList.

This is patch 3 of 4, to add support for COPYPRIVATE in Flang.
Original PR: #73128


Patch is 32.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80485.diff

5 Files Affected:

  • (modified) flang/include/flang/Lower/AbstractConverter.h (+3)
  • (modified) flang/lib/Lower/Bridge.cpp (+79-58)
  • (modified) flang/lib/Lower/OpenMP.cpp (+168-5)
  • (removed) flang/test/Lower/OpenMP/Todo/copyprivate.f90 (-13)
  • (added) flang/test/Lower/OpenMP/copyprivate.f90 (+164)
diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h
index c19dcbdcdb390..48804c327e1c7 100644
--- a/flang/include/flang/Lower/AbstractConverter.h
+++ b/flang/include/flang/Lower/AbstractConverter.h
@@ -120,6 +120,9 @@ class AbstractConverter {
       const Fortran::semantics::Symbol &sym,
       mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) = 0;
 
+  virtual void copyVar(mlir::Location loc, mlir::Value dst,
+                       mlir::Value src) = 0;
+
   /// For a given symbol, check if it is present in the inner-most
   /// level of the symbol map.
   virtual bool isPresentShallowLookup(Fortran::semantics::Symbol &sym) = 0;
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 8006b9b426f4d..04d09c4848491 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -743,6 +743,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         });
   }
 
+  void copyVar(mlir::Location loc, mlir::Value dst,
+               mlir::Value src) override final {
+    copyVarHLFIR(loc, dst, src);
+  }
+
   void copyHostAssociateVar(
       const Fortran::semantics::Symbol &sym,
       mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) override final {
@@ -777,64 +782,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       rhs_sb = &hsb;
     }
 
-    mlir::Location loc = genLocation(sym.name());
-
-    if (lowerToHighLevelFIR()) {
-      hlfir::Entity lhs{lhs_sb->getAddr()};
-      hlfir::Entity rhs{rhs_sb->getAddr()};
-      // Temporary_lhs is set to true in hlfir.assign below to avoid user
-      // assignment to be used and finalization to be called on the LHS.
-      // This may or may not be correct but mimics the current behaviour
-      // without HLFIR.
-      auto copyData = [&](hlfir::Entity l, hlfir::Entity r) {
-        // Dereference RHS and load it if trivial scalar.
-        r = hlfir::loadTrivialScalar(loc, *builder, r);
-        builder->create<hlfir::AssignOp>(
-            loc, r, l,
-            /*isWholeAllocatableAssignment=*/false,
-            /*keepLhsLengthInAllocatableAssignment=*/false,
-            /*temporary_lhs=*/true);
-      };
-      if (lhs.isAllocatable()) {
-        // Deep copy allocatable if it is allocated.
-        // Note that when allocated, the RHS is already allocated with the LHS
-        // shape for copy on entry in createHostAssociateVarClone.
-        // For lastprivate, this assumes that the RHS was not reallocated in
-        // the OpenMP region.
-        lhs = hlfir::derefPointersAndAllocatables(loc, *builder, lhs);
-        mlir::Value addr = hlfir::genVariableRawAddress(loc, *builder, lhs);
-        mlir::Value isAllocated = builder->genIsNotNullAddr(loc, addr);
-        builder->genIfThen(loc, isAllocated)
-            .genThen([&]() {
-              // Copy the DATA, not the descriptors.
-              copyData(lhs, rhs);
-            })
-            .end();
-      } else if (lhs.isPointer()) {
-        // Set LHS target to the target of RHS (do not copy the RHS
-        // target data into the LHS target storage).
-        auto loadVal = builder->create<fir::LoadOp>(loc, rhs);
-        builder->create<fir::StoreOp>(loc, loadVal, lhs);
-      } else {
-        // Non ALLOCATABLE/POINTER variable. Simple DATA copy.
-        copyData(lhs, rhs);
-      }
-    } else {
-      fir::ExtendedValue lhs = symBoxToExtendedValue(*lhs_sb);
-      fir::ExtendedValue rhs = symBoxToExtendedValue(*rhs_sb);
-      mlir::Type symType = genType(sym);
-      if (auto seqTy = symType.dyn_cast<fir::SequenceType>()) {
-        Fortran::lower::StatementContext stmtCtx;
-        Fortran::lower::createSomeArrayAssignment(*this, lhs, rhs, localSymbols,
-                                                  stmtCtx);
-        stmtCtx.finalizeAndReset();
-      } else if (lhs.getBoxOf<fir::CharBoxValue>()) {
-        fir::factory::CharacterExprHelper{*builder, loc}.createAssign(lhs, rhs);
-      } else {
-        auto loadVal = builder->create<fir::LoadOp>(loc, fir::getBase(rhs));
-        builder->create<fir::StoreOp>(loc, loadVal, fir::getBase(lhs));
-      }
-    }
+    copyVar(sym, *lhs_sb, *rhs_sb);
 
     if (copyAssignIP && copyAssignIP->isSet() &&
         sym.test(Fortran::semantics::Symbol::Flag::OmpLastPrivate)) {
@@ -1092,6 +1040,79 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     return true;
   }
 
+  void copyVar(const Fortran::semantics::Symbol &sym,
+               const Fortran::lower::SymbolBox &lhs_sb,
+               const Fortran::lower::SymbolBox &rhs_sb) {
+    mlir::Location loc = genLocation(sym.name());
+    if (lowerToHighLevelFIR())
+      copyVarHLFIR(loc, lhs_sb.getAddr(), rhs_sb.getAddr());
+    else
+      copyVarFIR(loc, sym, lhs_sb, rhs_sb);
+  }
+
+  void copyVarHLFIR(mlir::Location loc, mlir::Value dst, mlir::Value src) {
+    assert(lowerToHighLevelFIR());
+    hlfir::Entity lhs{dst};
+    hlfir::Entity rhs{src};
+    // Temporary_lhs is set to true in hlfir.assign below to avoid user
+    // assignment to be used and finalization to be called on the LHS.
+    // This may or may not be correct but mimics the current behaviour
+    // without HLFIR.
+    auto copyData = [&](hlfir::Entity l, hlfir::Entity r) {
+      // Dereference RHS and load it if trivial scalar.
+      r = hlfir::loadTrivialScalar(loc, *builder, r);
+      builder->create<hlfir::AssignOp>(
+          loc, r, l,
+          /*isWholeAllocatableAssignment=*/false,
+          /*keepLhsLengthInAllocatableAssignment=*/false,
+          /*temporary_lhs=*/true);
+    };
+    if (lhs.isAllocatable()) {
+      // Deep copy allocatable if it is allocated.
+      // Note that when allocated, the RHS is already allocated with the LHS
+      // shape for copy on entry in createHostAssociateVarClone.
+      // For lastprivate, this assumes that the RHS was not reallocated in
+      // the OpenMP region.
+      lhs = hlfir::derefPointersAndAllocatables(loc, *builder, lhs);
+      mlir::Value addr = hlfir::genVariableRawAddress(loc, *builder, lhs);
+      mlir::Value isAllocated = builder->genIsNotNullAddr(loc, addr);
+      builder->genIfThen(loc, isAllocated)
+          .genThen([&]() {
+            // Copy the DATA, not the descriptors.
+            copyData(lhs, rhs);
+          })
+          .end();
+    } else if (lhs.isPointer()) {
+      // Set LHS target to the target of RHS (do not copy the RHS
+      // target data into the LHS target storage).
+      auto loadVal = builder->create<fir::LoadOp>(loc, rhs);
+      builder->create<fir::StoreOp>(loc, loadVal, lhs);
+    } else {
+      // Non ALLOCATABLE/POINTER variable. Simple DATA copy.
+      copyData(lhs, rhs);
+    }
+  }
+
+  void copyVarFIR(mlir::Location loc, const Fortran::semantics::Symbol &sym,
+                  const Fortran::lower::SymbolBox &lhs_sb,
+                  const Fortran::lower::SymbolBox &rhs_sb) {
+    assert(!lowerToHighLevelFIR());
+    fir::ExtendedValue lhs = symBoxToExtendedValue(lhs_sb);
+    fir::ExtendedValue rhs = symBoxToExtendedValue(rhs_sb);
+    mlir::Type symType = genType(sym);
+    if (auto seqTy = symType.dyn_cast<fir::SequenceType>()) {
+      Fortran::lower::StatementContext stmtCtx;
+      Fortran::lower::createSomeArrayAssignment(*this, lhs, rhs, localSymbols,
+                                                stmtCtx);
+      stmtCtx.finalizeAndReset();
+    } else if (lhs.getBoxOf<fir::CharBoxValue>()) {
+      fir::factory::CharacterExprHelper{*builder, loc}.createAssign(lhs, rhs);
+    } else {
+      auto loadVal = builder->create<fir::LoadOp>(loc, fir::getBase(rhs));
+      builder->create<fir::StoreOp>(loc, loadVal, fir::getBase(lhs));
+    }
+  }
+
   /// Map a block argument to a result or dummy symbol. This is not the
   /// definitive mapping. The specification expression have not been lowered
   /// yet. The final mapping will be done using this pre-mapping in
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 34ebf2e91b6d8..7011cfe58f785 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -22,6 +22,7 @@
 #include "flang/Optimizer/Builder/BoxValue.h"
 #include "flang/Optimizer/Builder/FIRBuilder.h"
 #include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
 #include "flang/Parser/dump-parse-tree.h"
 #include "flang/Parser/parse-tree.h"
@@ -592,6 +593,10 @@ class ClauseProcessor {
   processAllocate(llvm::SmallVectorImpl<mlir::Value> &allocatorOperands,
                   llvm::SmallVectorImpl<mlir::Value> &allocateOperands) const;
   bool processCopyin() const;
+  bool processCopyPrivate(
+      mlir::Location currentLocation,
+      llvm::SmallVectorImpl<mlir::Value> &copyPrivateVars,
+      llvm::SmallVectorImpl<mlir::Attribute> &copyPrivateFuncs) const;
   bool processDepend(llvm::SmallVectorImpl<mlir::Attribute> &dependTypeOperands,
                      llvm::SmallVectorImpl<mlir::Value> &dependOperands) const;
   bool
@@ -1160,6 +1165,102 @@ class ReductionProcessor {
   }
 };
 
+/// Class that extracts information from the specified type.
+class TypeInfo {
+public:
+  TypeInfo(mlir::Type ty) { typeScan(ty); }
+
+  // Returns the length of character types.
+  std::optional<fir::CharacterType::LenType> getCharLength() const {
+    return charLen;
+  }
+
+  // Returns the shape of array types.
+  const llvm::SmallVector<int64_t> &getShape() const { return shape; }
+
+  // Is the type inside a box?
+  bool isBox() const { return inBox; }
+
+private:
+  void typeScan(mlir::Type type);
+
+  std::optional<fir::CharacterType::LenType> charLen;
+  llvm::SmallVector<int64_t> shape;
+  bool inBox = false;
+};
+
+void TypeInfo::typeScan(mlir::Type ty) {
+  if (auto sty = mlir::dyn_cast<fir::SequenceType>(ty)) {
+    assert(shape.empty() && !sty.getShape().empty());
+    shape = llvm::SmallVector<int64_t>(sty.getShape());
+    typeScan(sty.getEleTy());
+  } else if (auto bty = mlir::dyn_cast<fir::BoxType>(ty)) {
+    inBox = true;
+    typeScan(bty.getEleTy());
+  } else if (auto cty = mlir::dyn_cast<fir::CharacterType>(ty)) {
+    charLen = cty.getLen();
+  } else if (auto hty = mlir::dyn_cast<fir::HeapType>(ty)) {
+    typeScan(hty.getEleTy());
+  } else if (auto pty = mlir::dyn_cast<fir::PointerType>(ty)) {
+    typeScan(pty.getEleTy());
+  }
+}
+
+// Create a function that performs a copy between two variables, compatible
+// with their types and attributes.
+static mlir::func::FuncOp
+createCopyFunc(mlir::Location loc, Fortran::lower::AbstractConverter &converter,
+               mlir::Type varType, fir::FortranVariableFlagsEnum varAttrs) {
+  fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+  mlir::ModuleOp module = builder.getModule();
+  mlir::Type eleTy = mlir::cast<fir::ReferenceType>(varType).getEleTy();
+  TypeInfo typeInfo(eleTy);
+  std::string copyFuncName =
+      fir::getTypeAsString(eleTy, builder.getKindMap(), "_copy");
+
+  if (auto decl = module.lookupSymbol<mlir::func::FuncOp>(copyFuncName))
+    return decl;
+
+  // create function
+  mlir::OpBuilder::InsertionGuard guard(builder);
+  mlir::OpBuilder modBuilder(module.getBodyRegion());
+  llvm::SmallVector<mlir::Type> argsTy = {varType, varType};
+  auto funcType = mlir::FunctionType::get(builder.getContext(), argsTy, {});
+  mlir::func::FuncOp funcOp =
+      modBuilder.create<mlir::func::FuncOp>(loc, copyFuncName, funcType);
+  funcOp.setVisibility(mlir::SymbolTable::Visibility::Private);
+  builder.createBlock(&funcOp.getRegion(), funcOp.getRegion().end(), argsTy,
+                      {loc, loc});
+  builder.setInsertionPointToStart(&funcOp.getRegion().back());
+  // generate body
+  fir::FortranVariableFlagsAttr attrs;
+  if (varAttrs != fir::FortranVariableFlagsEnum::None)
+    attrs = fir::FortranVariableFlagsAttr::get(builder.getContext(), varAttrs);
+  llvm::SmallVector<mlir::Value> typeparams;
+  if (typeInfo.getCharLength().has_value()) {
+    mlir::Value charLen = builder.createIntegerConstant(
+        loc, builder.getCharacterLengthType(), *typeInfo.getCharLength());
+    typeparams.push_back(charLen);
+  }
+  mlir::Value shape;
+  if (!typeInfo.isBox() && !typeInfo.getShape().empty()) {
+    llvm::SmallVector<mlir::Value> extents;
+    for (auto extent : typeInfo.getShape())
+      extents.push_back(
+          builder.createIntegerConstant(loc, builder.getIndexType(), extent));
+    shape = builder.create<fir::ShapeOp>(loc, extents);
+  }
+  auto declDst = builder.create<hlfir::DeclareOp>(loc, funcOp.getArgument(0),
+                                                  copyFuncName + "_dst", shape,
+                                                  typeparams, attrs);
+  auto declSrc = builder.create<hlfir::DeclareOp>(loc, funcOp.getArgument(1),
+                                                  copyFuncName + "_src", shape,
+                                                  typeparams, attrs);
+  converter.copyVar(loc, declDst.getBase(), declSrc.getBase());
+  builder.create<mlir::func::ReturnOp>(loc);
+  return funcOp;
+}
+
 static mlir::omp::ScheduleModifier
 translateScheduleModifier(const Fortran::parser::OmpScheduleModifierType &m) {
   switch (m.v) {
@@ -1740,6 +1841,62 @@ bool ClauseProcessor::processCopyin() const {
   return hasCopyin;
 }
 
+bool ClauseProcessor::processCopyPrivate(
+    mlir::Location currentLocation,
+    llvm::SmallVectorImpl<mlir::Value> &copyPrivateVars,
+    llvm::SmallVectorImpl<mlir::Attribute> &copyPrivateFuncs) const {
+  auto addCopyPrivateVar = [&](Fortran::semantics::Symbol *sym) {
+    mlir::Value symVal = converter.getSymbolAddress(*sym);
+    auto declOp = symVal.getDefiningOp<hlfir::DeclareOp>();
+    if (!declOp)
+      fir::emitFatalError(currentLocation,
+                          "COPYPRIVATE is supported only in HLFIR mode");
+    symVal = declOp.getBase();
+    mlir::Type symType = symVal.getType();
+    fir::FortranVariableFlagsEnum attrs =
+        declOp.getFortranAttrs().has_value()
+            ? *declOp.getFortranAttrs()
+            : fir::FortranVariableFlagsEnum::None;
+    mlir::Value cpVar = symVal;
+
+    // CopyPrivate variables must be passed by reference. However, in the case
+    // of assumed shapes/vla the type is not a !fir.ref, but a !fir.box.
+    // In these cases to retrieve the appropriate !fir.ref<!fir.box<...>> to
+    // access the data we need we must perform an alloca and then store to it
+    // and retrieve the data from the new alloca.
+    if (mlir::isa<fir::BaseBoxType>(symType)) {
+      fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+      auto alloca = builder.create<fir::AllocaOp>(currentLocation, symType);
+      builder.create<fir::StoreOp>(currentLocation, symVal, alloca);
+      cpVar = alloca;
+    }
+
+    copyPrivateVars.push_back(cpVar);
+    mlir::func::FuncOp funcOp =
+        createCopyFunc(currentLocation, converter, cpVar.getType(), attrs);
+    copyPrivateFuncs.push_back(mlir::SymbolRefAttr::get(funcOp));
+  };
+
+  bool hasCopyPrivate = findRepeatableClause<ClauseTy::Copyprivate>(
+      [&](const ClauseTy::Copyprivate *copyPrivateClause,
+          const Fortran::parser::CharBlock &) {
+        const Fortran::parser::OmpObjectList &ompObjectList =
+            copyPrivateClause->v;
+        for (const Fortran::parser::OmpObject &ompObject : ompObjectList.v) {
+          Fortran::semantics::Symbol *sym = getOmpObjectSymbol(ompObject);
+          if (const auto *commonDetails =
+                  sym->detailsIf<Fortran::semantics::CommonBlockDetails>()) {
+            for (const auto &mem : commonDetails->objects())
+              addCopyPrivateVar(&*mem);
+            break;
+          }
+          addCopyPrivateVar(sym);
+        }
+      });
+
+  return hasCopyPrivate;
+}
+
 bool ClauseProcessor::processDepend(
     llvm::SmallVectorImpl<mlir::Attribute> &dependTypeOperands,
     llvm::SmallVectorImpl<mlir::Value> &dependOperands) const {
@@ -2482,19 +2639,24 @@ genSingleOp(Fortran::lower::AbstractConverter &converter,
             const Fortran::parser::OmpClauseList &endClauseList) {
   llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
   llvm::SmallVector<mlir::Value> copyPrivateVars;
+  llvm::SmallVector<mlir::Attribute> copyPrivateFuncs;
   mlir::UnitAttr nowaitAttr;
 
   ClauseProcessor cp(converter, beginClauseList);
   cp.processAllocate(allocatorOperands, allocateOperands);
-  cp.processTODO<Fortran::parser::OmpClause::Copyprivate>(
-      currentLocation, llvm::omp::Directive::OMPD_single);
 
-  ClauseProcessor(converter, endClauseList).processNowait(nowaitAttr);
+  ClauseProcessor ecp(converter, endClauseList);
+  ecp.processNowait(nowaitAttr);
+  ecp.processCopyPrivate(currentLocation, copyPrivateVars, copyPrivateFuncs);
 
   return genOpWithBody<mlir::omp::SingleOp>(
       converter, eval, genNested, currentLocation,
       /*outerCombined=*/false, &beginClauseList, allocateOperands,
-      allocatorOperands, copyPrivateVars, /*copyPrivateFuncs=*/nullptr,
+      allocatorOperands, copyPrivateVars,
+      copyPrivateFuncs.empty()
+          ? nullptr
+          : mlir::ArrayAttr::get(converter.getFirOpBuilder().getContext(),
+                                 copyPrivateFuncs),
       nowaitAttr);
 }
 
@@ -3369,7 +3531,8 @@ genOMP(Fortran::lower::AbstractConverter &converter,
 
   for (const auto &clause : endClauseList.v) {
     mlir::Location clauseLocation = converter.genLocation(clause.source);
-    if (!std::get_if<Fortran::parser::OmpClause::Nowait>(&clause.u))
+    if (!std::get_if<Fortran::parser::OmpClause::Nowait>(&clause.u) &&
+        !std::get_if<Fortran::parser::OmpClause::Copyprivate>(&clause.u))
       TODO(clauseLocation, "OpenMP Block construct clause");
   }
 
diff --git a/flang/test/Lower/OpenMP/Todo/copyprivate.f90 b/flang/test/Lower/OpenMP/Todo/copyprivate.f90
deleted file mode 100644
index 0d871427ce60f..0000000000000
--- a/flang/test/Lower/OpenMP/Todo/copyprivate.f90
+++ /dev/null
@@ -1,13 +0,0 @@
-! 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: OpenMP Block construct clause
-subroutine sb
-  integer, save :: a
-  !$omp threadprivate(a)
-  !$omp parallel
-  !$omp single
-  a = 3
-  !$omp end single copyprivate(a)
-  !$omp end parallel
-end subroutine
diff --git a/flang/test/Lower/OpenMP/copyprivate.f90 b/flang/test/Lower/OpenMP/copyprivate.f90
new file mode 100644
index 0000000000000..9b76a996ef3e1
--- /dev/null
+++ b/flang/test/Lower/OpenMP/copyprivate.f90
@@ -0,0 +1,164 @@
+! Test COPYPRIVATE.
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+!CHECK-DAG: func private @_copy_i64(%{{.*}}: !fir.ref<i64>, %{{.*}}: !fir.ref<i64>)
+!CHECK-DAG: func private @_copy_f32(%{{.*}}: !fir.ref<f32>, %{{.*}}: !fir.ref<f32>)
+!CHECK-DAG: func private @_copy_f64(%{{.*}}: !fir.ref<f64>, %{{.*}}: !fir.ref<f64>)
+!CHECK-DAG: func private @_copy_z32(%{{.*}}: !fir.ref<!fir.complex<4>>, %{{.*}}: !fir.ref<!fir.complex<4>>)
+!CHECK-DAG: func private @_copy_z64(%{{.*}}: !fir.ref<!fir.complex<8>>, %{{.*}}: !fir.ref<!fir.complex<8>>)
+!CHECK-DAG: func private @_copy_l32(%{{.*}}: !fir.ref<!fir.logical<4>>, %{{.*}}: !fir.ref<!fir.logical<4>>)
+!CHECK-DAG: func private @_copy_l64(%{{.*}}: !fir.ref<!fir.logical<8>>, %{{.*}}: !fir.ref<!fir.logical<8>>)
+!CHECK-DAG: func private @_copy_c8x3(%{{.*}}: !fir.ref<!fir.char<1,3>>, %{{.*}}: !fir.ref<!fir.char<1,3>>)
+!CHECK-DAG: func private @_copy_c8x8(%{{.*}}: !fir.ref<!fir.char<1,8>>, %{{.*}}: !fir.ref<!fir.char<1,8>>)
+!CHECK-DAG: func private @_copy_c16x8(%{{.*}}: !fir.ref<!fir.char<2,8>>, %{{.*}}: !fir.ref<!fir.char<2,8>>)
+
+!CHECK-DAG: func private @_copy_box_Uxi32(%{{.*}}: !fir.ref<!fir.box<!fir.array<?xi32>>>, %{{.*}}: !fir.ref<!fir.box<!fir.array<?xi32>>>)
+!CHECK-DAG: func private @_copy_10xi32(%{{.*}}: !fir.ref<!fir.array<10xi32>>, %{{.*}}: !fir.ref<!fir.array<10xi32>>)
+!CHECK-DAG: func private @_copy_3x4xi32(%{{.*}}: !fir.ref<!fir.array<3x4xi32>>, %{{.*}}: !fir.ref<!fir.array<3x4xi32>>)
+!CHECK-DAG: func private @_copy_10xf32(%{{.*}}: !fir.ref<!fir.array<10xf32>>, %{{.*}}: !fir.ref<!fir.array<10xf32>>)
+!CHECK-DAG: func private @_copy_3x4xz32(%{{.*}}: !fir.ref<!fir.array<3x4x!fir.complex<4>>>, %{{.*}}: !fir.ref<!fir.array<3x4x!fir.complex<4>>>)
...
[truncated]

Comment on lines +1043 to +1116
void copyVar(const Fortran::semantics::Symbol &sym,
const Fortran::lower::SymbolBox &lhs_sb,
const Fortran::lower::SymbolBox &rhs_sb) {
mlir::Location loc = genLocation(sym.name());
if (lowerToHighLevelFIR())
copyVarHLFIR(loc, lhs_sb.getAddr(), rhs_sb.getAddr());
else
copyVarFIR(loc, sym, lhs_sb, rhs_sb);
}

void copyVarHLFIR(mlir::Location loc, mlir::Value dst, mlir::Value src) {
assert(lowerToHighLevelFIR());
hlfir::Entity lhs{dst};
hlfir::Entity rhs{src};
// Temporary_lhs is set to true in hlfir.assign below to avoid user
// assignment to be used and finalization to be called on the LHS.
// This may or may not be correct but mimics the current behaviour
// without HLFIR.
auto copyData = [&](hlfir::Entity l, hlfir::Entity r) {
// Dereference RHS and load it if trivial scalar.
r = hlfir::loadTrivialScalar(loc, *builder, r);
builder->create<hlfir::AssignOp>(
loc, r, l,
/*isWholeAllocatableAssignment=*/false,
/*keepLhsLengthInAllocatableAssignment=*/false,
/*temporary_lhs=*/true);
};
if (lhs.isAllocatable()) {
// Deep copy allocatable if it is allocated.
// Note that when allocated, the RHS is already allocated with the LHS
// shape for copy on entry in createHostAssociateVarClone.
// For lastprivate, this assumes that the RHS was not reallocated in
// the OpenMP region.
lhs = hlfir::derefPointersAndAllocatables(loc, *builder, lhs);
mlir::Value addr = hlfir::genVariableRawAddress(loc, *builder, lhs);
mlir::Value isAllocated = builder->genIsNotNullAddr(loc, addr);
builder->genIfThen(loc, isAllocated)
.genThen([&]() {
// Copy the DATA, not the descriptors.
copyData(lhs, rhs);
})
.end();
} else if (lhs.isPointer()) {
// Set LHS target to the target of RHS (do not copy the RHS
// target data into the LHS target storage).
auto loadVal = builder->create<fir::LoadOp>(loc, rhs);
builder->create<fir::StoreOp>(loc, loadVal, lhs);
} else {
// Non ALLOCATABLE/POINTER variable. Simple DATA copy.
copyData(lhs, rhs);
}
}

void copyVarFIR(mlir::Location loc, const Fortran::semantics::Symbol &sym,
const Fortran::lower::SymbolBox &lhs_sb,
const Fortran::lower::SymbolBox &rhs_sb) {
assert(!lowerToHighLevelFIR());
fir::ExtendedValue lhs = symBoxToExtendedValue(lhs_sb);
fir::ExtendedValue rhs = symBoxToExtendedValue(rhs_sb);
mlir::Type symType = genType(sym);
if (auto seqTy = symType.dyn_cast<fir::SequenceType>()) {
Fortran::lower::StatementContext stmtCtx;
Fortran::lower::createSomeArrayAssignment(*this, lhs, rhs, localSymbols,
stmtCtx);
stmtCtx.finalizeAndReset();
} else if (lhs.getBoxOf<fir::CharBoxValue>()) {
fir::factory::CharacterExprHelper{*builder, loc}.createAssign(lhs, rhs);
} else {
auto loadVal = builder->create<fir::LoadOp>(loc, fir::getBase(rhs));
builder->create<fir::StoreOp>(loc, loadVal, fir::getBase(lhs));
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be in OpenMP.cpp? It would remove the need to add a function to the converter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be possible to move this to OpenMP.cpp, but this would mean duplicating around 40 lines of code.
The copyVarHLFIR() code was extracted from copyHostAssociateVar(), that now calls copyVar() instead.

Can we keep it in the converter to avoid code duplication?

@luporl luporl changed the base branch from users/luporl/copypriv-sema-omp to main February 16, 2024 19:50
@luporl luporl force-pushed the luporl-copypriv-flang branch from 06f0aa9 to 4fb1267 Compare February 16, 2024 19:58
@luporl
Copy link
Contributor Author

luporl commented Feb 19, 2024

ping

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

typeScan(hty.getEleTy());
} else if (auto pty = mlir::dyn_cast<fir::PointerType>(ty)) {
typeScan(pty.getEleTy());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should there be an assert here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Add initial handling of OpenMP copyprivate clause in Flang.

When lowering copyprivate, Flang generates the copy function
needed by each variable and builds the appropriate
omp.single's CopyPrivateVarList.

This is patch 3 of 4, to add support for COPYPRIVATE in Flang.
Original PR: llvm#73128
@luporl luporl force-pushed the luporl-copypriv-flang branch from 2e660a0 to 95131ec Compare February 21, 2024 17:06
@luporl luporl merged commit e50a231 into llvm:main Feb 21, 2024
@luporl luporl deleted the luporl-copypriv-flang branch February 21, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants