[Mlir-commits] [flang] [mlir] [mlir][OpenMP] Add copyprivate support to omp.single (PR #80477)

[Leandro Lupori]

================
@@ -505,6 +505,107 @@ static LogicalResult verifyReductionVarList(Operation *op,
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// Parser, printer and verifier for CopyPrivateVarList
+//===----------------------------------------------------------------------===//
+
+/// copyprivate-entry-list ::= copyprivate-entry
+///                          | copyprivate-entry-list `,` copyprivate-entry
+/// copyprivate-entry ::= ssa-id `->` symbol-ref `:` type
+static ParseResult parseCopyPrivateVarList(
+    OpAsmParser &parser,
+    SmallVectorImpl<OpAsmParser::UnresolvedOperand> &operands,
+    SmallVectorImpl<Type> &types, ArrayAttr &copyPrivateSymbols) {
+  SmallVector<SymbolRefAttr> copyPrivateFuncsVec;
+  if (failed(parser.parseCommaSeparatedList([&]() {
+        if (parser.parseOperand(operands.emplace_back()) ||
+            parser.parseArrow() ||
+            parser.parseAttribute(copyPrivateFuncsVec.emplace_back()) ||
+            parser.parseColonType(types.emplace_back()))
+          return failure();
+        return success();
+      })))
+    return failure();
+  SmallVector<Attribute> copyPrivateFuncs(copyPrivateFuncsVec.begin(),
+                                          copyPrivateFuncsVec.end());
+  copyPrivateSymbols = ArrayAttr::get(parser.getContext(), copyPrivateFuncs);
+  return success();
+}
+
+/// Print CopyPrivate clause
+static void printCopyPrivateVarList(OpAsmPrinter &p, Operation *op,
+                                    OperandRange copyPrivateVars,
+                                    TypeRange copyPrivateTypes,
+                                    std::optional<ArrayAttr> copyPrivateFuncs) {
+  assert(copyPrivateFuncs.has_value() || copyPrivateVars.empty());
+  for (unsigned i = 0, e = copyPrivateVars.size(); i < e; ++i) {
+    if (i != 0)
+      p << ", ";
+    p << copyPrivateVars[i] << " -> " << (*copyPrivateFuncs)[i] << " : "
+      << copyPrivateTypes[i];
+  }
+}
+
+/// Verifies CopyPrivate Clause
+static LogicalResult
+verifyCopyPrivateVarList(Operation *op, OperandRange copyPrivateVars,
+                         std::optional<ArrayAttr> copyPrivateFuncs) {
+  if (!copyPrivateVars.empty()) {
+    if (!copyPrivateFuncs || copyPrivateFuncs->size() != copyPrivateVars.size())
+      return op->emitOpError() << "expected as many copyPrivate functions as "
+                                  "copyPrivate variables";
+  } else {
+    if (copyPrivateFuncs)
+      return op->emitOpError() << "unexpected copyPrivate functions";
+    return success();
+  }
+
+  for (auto args : llvm::zip(copyPrivateVars, *copyPrivateFuncs)) {
+    auto symbolRef = llvm::cast<SymbolRefAttr>(std::get<1>(args));
+    std::optional<std::variant<mlir::func::FuncOp, mlir::LLVM::LLVMFuncOp>>
+        funcOp;
----------------
luporl wrote:

With the current implementation, changing the copy functions to `CopyPrivateDeclare` ops would introduce an issue. Unlike reduction and privatization, which seems to inline the operation's regions within the OpenMP construct, copyprivate needs to pass a function pointer to `OpenMPIRBuilder::createCopyPrivate`.

I guess the operation's region would need to be used to create a function, unless there is another way of using a region as a function that I'm not aware. But later on, when we implement the optimization of calling `__kmpc_copyprivate` only once, by using an array of pointers to perform all copies within a single function, these created functions wouldn't be needed anymore. Instead, it would be possible to inline the regions of the `CopyPrivateDeclare` ops into that single function.

So, WDYT of leaving this change for a future patch? Otherwise, I think it would be better to implement the optimization already, to avoid putting effort in code that would later be discarded.

https://github.com/llvm/llvm-project/pull/80477