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

[Kiran Chandramohan 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());
----------------
kiranchandramohan wrote:

You should feel free to proceed with the approval of @agozillon and @skatrak. You can set up a quick call with them and describe your approach and come to a conclusion. You can report the outcomes somewhere in this patch or write in discourse. 

> Inside DeclMapperOp's region I've introduce a new DeclMapperInfoOp to which I've attached the MapClause. Not having any MapClause explicitly associated seemed weird to me, also walking through the region collecting all the MapInfoOps for processing in various places in the code base seemed like a bad design to me.

The idea would generally be to inline the whole declare mapper operation region, replacing the block_arg with the real variable that is going to be mapped. Would an alloca always be created for all kinds of mappings? If not committing to an alloca might mean you have to delete it in some circumstances.

> We can drop the entire DeclMapperOp including the region once it reaches OpenMPTOLLVMIRTranslation.

As in just drop it without using it? Or using it create the `@.omp_mapper._ZTS1T.deep_copy` function for the declare mapper (`#pragma omp declare mapper(deep_copy : T abc) map(abc, abc.ptr[ : abc.buf_size])`) in the C example you gave below.

You have not talked about the following two points.
```
-> where we create the map_entries for the relevant operations (like target) for which the declare mapper implicitly applies. Currently, this is done during lowering. But in this patch you have solely focused on creating the declare mapper.
-> where the composition of map-types (map-type decay) from the map clause of declare mapper and the map clause of the relevant operation (like target) happens
```

Couple of other things that came to my mind :
-> Since declare mappers are in the specification section, it can also occur in modules. We have not added code to propagate it to the module file. If this is not urgent for you, I can fix it sometime.
-> You should test the case where the declare mapper is in the host subroutine
```
subroutine A
type t
end type
declare mapper(t)
contains
subroutine B
!$omp target 
! use a var of type t
!$omp end target
end subroutine
```

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