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

[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());
----------------
agozillon wrote:

I don't have much insight into how the (soon to be) declare mapper portion of the OMPIRBuilder or omp.private/declare_reduction work unfortunately, and I am nowhere near an MLIR guru, still at a beginner level I'd wager, so take my comment with a grain of salt :-) 

I do think it would be ideal to maintain the correct scoping if we can, as the specification seems to be quite clear on it, although, I imagine if there is significant difficulties in doing so at an MLIR level as long as semantically it is the same to a user (i.e. no issues / bugs), then the specification doesn't really have room to complain as we're in the land of implementation details. However, if we can maintain the scoping as it seems to be suggested is possible, that'd be awesome :-)

Otherwise, I do quite like the lambda-esque approach conceptually as it does seem to achieve the desired results as far as scoping goes, and it is tidy in the sense that it can be shared across multiple mappings of the variable/type the mapper has been declared for quite trivially and follows the whole concept that declare mapper is effectively creating a specification/template of the desired mapping. Saying that, I am not against the simpler method either, I am not sure which would be easier to perform transformations/optimisations on if we ever wished to. 

As far as the lowering goes, I don't think it'd be too difficult in either case. The main pass that does things with map information is as you've mentioned MapInfoFinalization, which I believe should only need minor tweaks, primarily to recognize declare mappers as something that contains map.info that needs walked over, you would also likely have to teach it to add any new maps it generates to the yield operation as well. So some tedium but nothing too egregious I think, and more than happy to help with it. I do not think there's anything incredibly complex as far as the OpenMP -> OpenMP dialect translation that occurs when we lower from FIR to LLVM dialect goes as far as the map info, so it'd likely just be a case of mimic'ng how other OMP operations that follow the same format work at that stage. For MLIR -> LLVM-IR, you'd likely still have to do a lot of the same legwork in either case with some differences I would imagine, as at the end of the day the map info and bounds information is just an information vehicle, it'll have to be converted into whatever the declare mapper portion of the OMPIRBuilder desires, which won't be the exact same as what use_device_addr does or what map does. 

But as with anything complex, there will most definitely be issues and unforeseen things you'll likely run into and I am not the soldier in the trenches so to speak, so I imagine you'll be able to assess the difficulties better than I can :-)  

P.S. I would love to see what Clang emits for a declare mapper in the IR if you have a spare blob of IR handy or a small sample program I can compile! Might help to give some further insight into what the end result for us will be (at least it'll help me in any case, will give me a better idea of what the expectations for what the MLIR -> LLVM-IR portion will need to do).

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