[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)

[Akash Banerjee via llvm-branch-commits]

================
@@ -2701,7 +2701,42 @@ static void
 genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
        semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
        const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
-  TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  lower::StatementContext stmtCtx;
+  const auto &spec =
+      std::get<parser::OmpDeclareMapperSpecifier>(declareMapperConstruct.t);
+  const auto &mapperName{std::get<std::optional<parser::Name>>(spec.t)};
+  const auto &varType{std::get<parser::TypeSpec>(spec.t)};
+  const auto &varName{std::get<parser::Name>(spec.t)};
+  assert(varType.declTypeSpec->category() ==
+             semantics::DeclTypeSpec::Category::TypeDerived &&
+         "Expected derived type");
+
+  std::string mapperNameStr;
+  if (mapperName.has_value())
+    mapperNameStr = mapperName->ToString();
+  else
+    mapperNameStr =
+        "default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+  mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+  firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+  auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+  auto varVal = firOpBuilder.createTemporaryAlloc(
+      converter.getCurrentLocation(), mlirType, varName.ToString());
----------------
TIFitis wrote:

>From what I understand, declare mapper directive doesn't simply apply those mapping rules as is to `%x` in your example, or at least not in the plain manner suggested.

A new mapper function is emitted, and a pointer to this function is passed on to the runtime kernel call for target. The OMPRTL takes offload_mappers as the last argument, which is an array of pointers to the user defined mapper funcs.

If we go with the approach you are suggesting above, we would have to decouple the `map clause` from the declare mapper directive and have floating `mapInfoOps` in its nested region. This seems like an off approach to me. Also, we would need to handle this as a special case everywhere we handle regular map clauses, including the several lowering stages and passes like `mapInfoFinalization`.

If we really want to enforce the scoping rule for the declare mapper `var`, perhaps we can do so in the verifier to check that `%var` has a single use, i.e, the `declMapperOp`. Let me know if that sounds reasonable and we can leave the `%var` declaration just above the `declMapperOp`.

As for the placement of the `declMapperOp` at the Module level or Func level, I am undecided. The `declMapperOp` is in some sense akin to a function declaration and will be lowered into one eventually in the llvm IR. We're just delaying that process until we lower to llvm IR, as it saves us unnecessary work, and also we can share the lowering between Clang and MLIR through the OMPIRBUilder.

With that being said, as we are not planning on expanding it into a function at the MLIR level, I am not opposed to leaving it inside the func it's defined in to retain scoping rules. We can do so by changing the `declMapperOp` to no longer declare a symbol for itself, to prevent MLIR from complaining about `ModuleOp` not being it's parent.
This would mean that later when lowering map clauses, instead of doing a sym_table lookup for the mapper, we would have to scan the code for `declMapperOps` instead and match. This may be slightly inefficient, but I don't think should be too much of an issue.

Let me know what are your thoughts on this and how you'd like me to proceed :)

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