[MLIR][LLVM] Add accessor for LLVMModule and invoke convertDialectAttributes on GlobalOps

[agozillon]

Summary

This patch seeks to do two things add an accessor method to
retrieve the ModuleTranslations contained LLVM Module for direct
usage by dialects that are being lowered to LLVM-IR. One particular
use case for this is in the OpenMP Dialect, when interfacing
with the OMPIRBuilder in certain cases it is useful to be able
to access the LLVM Module directly.

The second is invoking convertDialectAttributes on GlobalOp's
so as to be able to lower dialect specific attributes that are
applied or lowered onto GlobalOp's.

[agozillon]

Perhaps these are unreasonable changes (or the wrong way to go about things), so please do mention if they are. I've found some use for these modifications in the OpenMP Dialect when lowering an attribute applied to globals (declare target) which utilises some shared methods between Clang and the OpenMP dialect to lower the attribute https://reviews.llvm.org/D149162 to LLVM-IR. The LLVM Module accessor is to help with interfacing with the shared functions and the conversion of attributes helps to have access to the attribute during lowering within the OpenMPToLLVMTranslation phase.

[ftynse]

Please add a test for translating attributes on globals.

[agozillon]

In D149279#4302755, @ftynse wrote:

Please add a test for translating attributes on globals.

Thank you very much @ftynse for the quick review!

I don't have the lowering that makes use of these changes available upstream just yet, so I unfortunately can't add a test at the moment (although the lowering, which I am in the process of making a phabricator review of will have tests that depend on these changes and will thus test them). Unless there is an alternative attribute (or method of testing) I can test with at the moment.

So I could either upstream this patch as-is without a test for the moment (but the phabricator reviews that it relies on for testing could take some time to reach acceptance themselves) or leave this patch in it's accepted state until the other phabricator reviews that depend on the change have reached an accepted state as well?

[ftynse]

I'm fine landing this.

mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h

public:
   llvm::GlobalValue *lookupGlobal(Operation *op) {
     return globalsMapping.lookup(op);
   }
 
   /// Returns the OpenMP IR builder associated with the LLVM IR module being
   /// constructed.
   llvm::OpenMPIRBuilder *getOpenMPBuilder();
 
+  /// Returns the LLVM module in which the IR is being constructed.
+  llvm::Module *getLLVMModule() { return llvmModule.get(); }
+
   /// Translates the given location.
   const llvm::DILocation *translateLoc(Location loc, llvm::DILocalScope *scope);
 
   /// Translates the given LLVM debug info metadata.
   llvm::Metadata *translateDebugInfo(LLVM::DINodeAttr attr);
 
   /// Translates the contents of the given block to LLVM IR using this
   /// translator. The LLVM IR basic block corresponding to the given block is

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

for (auto symbolAndPriority : range) {
           std::get<0>(symbolAndPriority).cast<FlatSymbolRefAttr>().getValue());
       appendGlobalFn(
           *llvmModule, f,
           std::get<1>(symbolAndPriority).cast<IntegerAttr>().getInt(),
           /*Data=*/nullptr);
     }
   }
 
+  for (auto op : getModuleBody(mlirModule).getOps<LLVM::GlobalOp>())
+    if (failed(convertDialectAttributes(op)))
+      return failure();
+
   return success();
 }
 
 /// Attempts to add an attribute identified by `key`, optionally with the given
 /// `value` to LLVM function `llvmFunc`. Reports errors at `loc` if any. If the
 /// attribute has a kind known to LLVM IR, create the attribute of this kind,
 /// otherwise keep it as a string attribute. Performs additional checks for
 /// attributes known to have or not have a value in order to avoid assertions