================
@@ -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:

> You should feel free to proceed with the approval of @agozillon and @skatrak.
Thanks for the go ahead, I'll consult with them before proceeding, and post any 
relevant findings here.

> > 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.

In case of DeclareMapper the mapping is handled in a ad-hoc basis. There is no 
real variable to be replaced later. The variable simply exists as a dummy to 
represent the mapping information for other real variables where the mapping 
would be used. However, this mapping isn't applied directly to those variables 
either, rather the runtime is notified that a mapper function exists for these 
variables.

> > 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.
Yes, I mean't each declareMapperOp would be resolved to a function like 
`@.omp_mapper._ZTS1T.deep_copy`. By dropping it, I mean't it's region won't be 
separately lowered.

> 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
> ```

I'll address the implicit mapping either in the DeclMapper lowering support for 
the mapClause or a separate PR soon after that.

The composition of mapClause AFAIK is likely handled in the OMPIRBuilder. If 
that's not the case, I'll address it in a future patch. 

> 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. 

Please feel free to do this if you're already familiar with what needs to be 
done, as I'm not. Otherwise, I can add it to my TODO list :)

-> 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
> ```

Thanks, I'll test this in the mapClause mapper support PR once I have it ready.

Many thanks for the review.



https://github.com/llvm/llvm-project/pull/117046
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to