[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
Akash Banerjee via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Dec 3 06:48:22 PST 2024
================
@@ -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
More information about the llvm-branch-commits
mailing list