[Mlir-commits] [mlir] [MLIR][OpenMP] Add canonical loop LLVM-IR lowering (PR #147069)

[Michael Kruse]

================
@@ -108,6 +110,41 @@ class ModuleTranslation {
     return blockMapping.lookup(block);
   }
 
+  /// Find the LLVM-IR loop that represents an MLIR loop.
+  llvm::CanonicalLoopInfo *lookupOMPLoop(omp::NewCliOp mlir) const {
----------------
Meinersbur wrote:

Post-commit reviews are very welcome!

Since we need a namespace for `omp.new_cli` mappings, a potential `StateStackFrameBase` would need to be the top of the ModuleTranslation/function's stack frame. That requires additional code during function creation, i.e. there is still OpenMP-specific handling in otherwise LLVM dialect-centric translation. As being mandatory, it would not be much different than adding the namespace-lookup object (be it a class derived from StateStackFrameBase or the DenseMap directly) as a member of the ModuleTranslation class.

There is an `LLVMTranslationInterface iface` field in ModuleTranslation which looked promising at first, but also only reacts when encountering a dialect's operation or attribute. 

Any other ideas? I could hide the namespace lookup object from `ModuleTranslation.h` by using the pImpl idiom.

The abstraction seems to have been broken anyway. ModuleTranslation already has a `getOpenMPBuilder()` member and has several `lookupXYZ` members. MLIR could have translated to the LLVM Dialect in an iterative loweing step, but this idea was not followed in a compromise in order to use the OpenMPIRBuilder. Hence, ModuleTranslation will always require some OpenMP-specific code.

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