[MLIR] Allow setting "CodeView" flag in LLVMIR translation on MSVC.

[scotttodd]

There's potential for a more generic solution here. I tried passing a ArrayRef<llvm::Module::ModuleFlagEntry> through: https://github.com/ScottTodd/llvm-project/commit/e276941578e8afd1f77380eceb9d76cd1b11b41c , but I think something using attributes on the Operation might be better. For projects not using memrefs, the "malloc" and "free" functions could also be optional.

[ftynse]

In D91365#2391787, @scotttodd wrote:

There's potential for a more generic solution here. I tried passing a ArrayRef<llvm::Module::ModuleFlagEntry> through: https://github.com/ScottTodd/llvm-project/commit/e276941578e8afd1f77380eceb9d76cd1b11b41c , but I think something using attributes on the Operation might be better. For projects not using memrefs, the "malloc" and "free" functions could also be optional.

Can we just add a triple as a module attribute similarly to what we already have for DataLayout? module attributes {llvm.data_layout = "E", llvm.triple = "x86-windows-msvc"} {...}. There is always at least the top-level module. One caveat is that we need to make sure LLVM doesn't exit(1) if it fails to parse the triple.

[rriddle]

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

957

Anything debug related should be in DebugTranslation.

[scotttodd]

Use LLVM Dialect attribute to pass llvm::Triple string to DebugTranslation.cpp.

[mehdi_amini]

Can you add a test for this?

[scotttodd]

In D91365#2391919, @ftynse wrote:

In D91365#2391787, @scotttodd wrote:

There's potential for a more generic solution here. I tried passing a ArrayRef<llvm::Module::ModuleFlagEntry> through: https://github.com/ScottTodd/llvm-project/commit/e276941578e8afd1f77380eceb9d76cd1b11b41c , but I think something using attributes on the Operation might be better. For projects not using memrefs, the "malloc" and "free" functions could also be optional.

Can we just add a triple as a module attribute similarly to what we already have for DataLayout? module attributes {llvm.data_layout = "E", llvm.triple = "x86-windows-msvc"} {...}. There is always at least the top-level module. One caveat is that we need to make sure LLVM doesn't exit(1) if it fails to parse the triple.

Yep, that looks good to me. Any suggestions on avoiding the exit(1)?

[mehdi_amini]

You may need a refactor like Alex did here: https://reviews.llvm.org/D85650 ; what about a factory method returning ErrorOr<Triple> ?

[scotttodd]

Add tests for llvm.target_triple attribute handling.

[scotttodd]

In D91365#2392031, @scotttodd wrote:

In D91365#2391919, @ftynse wrote:

In D91365#2391787, @scotttodd wrote:

There's potential for a more generic solution here. I tried passing a ArrayRef<llvm::Module::ModuleFlagEntry> through: https://github.com/ScottTodd/llvm-project/commit/e276941578e8afd1f77380eceb9d76cd1b11b41c , but I think something using attributes on the Operation might be better. For projects not using memrefs, the "malloc" and "free" functions could also be optional.

Can we just add a triple as a module attribute similarly to what we already have for DataLayout? module attributes {llvm.data_layout = "E", llvm.triple = "x86-windows-msvc"} {...}. There is always at least the top-level module. One caveat is that we need to make sure LLVM doesn't exit(1) if it fails to parse the triple.

Yep, that looks good to me. Any suggestions on avoiding the exit(1)?

Looked through the source and tried some bogus values, I don't think LLVM crashes if it fails to parse into a meaningful Triple:
https://github.com/llvm/llvm-project/blob/master/llvm/unittests/ADT/TripleTest.cpp
https://github.com/llvm/llvm-project/blob/4726a402a32a0553cf4da8c59646d8bc99c08383/llvm/lib/Support/Triple.cpp#L737-L773

[mehdi_amini]

LGTM

[ftynse]

Thanks!

[scotttodd]

Thanks for the reviews! Can someone land this for me? (no commit access)

mlir/include/mlir/Target/LLVMIR.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_TARGET_LLVMIR_H
 #define MLIR_TARGET_LLVMIR_H
 
 #include "mlir/Support/LLVM.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 #include <memory>
 
 // Forward-declare LLVM classes.
 namespace llvm {
 class LLVMContext;
 class Module;
 } // namespace llvm
 
 namespace mlir {
 
 class OwningModuleRef;
 class MLIRContext;
 class ModuleOp;
 
 /// Convert the given MLIR module into LLVM IR.  The LLVM context is extracted
 /// from the registered LLVM IR dialect.  In case of error, report it
 /// to the error handler registered with the MLIR context, if any (obtained from
 /// the MLIR module), and return `nullptr`.
 std::unique_ptr<llvm::Module>
 translateModuleToLLVMIR(ModuleOp m, llvm::LLVMContext &llvmContext,
-                        StringRef name = "LLVMDialectModule");
+                        StringRef name = "LLVMDialectModule",
+                        llvm::Triple configTriple = llvm::Triple());
 
 /// Convert the given LLVM module into MLIR's LLVM dialect.  The LLVM context is
 /// extracted from the registered LLVM IR dialect. In case of error, report it
 /// to the error handler registered with the MLIR context, if any (obtained from
 /// the MLIR module), and return `{}`.
 OwningModuleRef
 translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
                         MLIRContext *context);
 
 } // namespace mlir
 
 #endif // MLIR_TARGET_LLVMIR_H

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

 
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/Dialect/LLVMIR/Transforms/LegalizeForExport.h"
 #include "mlir/IR/Block.h"
 #include "mlir/IR/Module.h"
 #include "mlir/IR/Value.h"
 #include "mlir/Target/LLVMIR/TypeTranslation.h"
 
+#include "llvm/ADT/Triple.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
 #include "llvm/IR/Value.h"
 
 namespace mlir {

 /// functions, basic blocks and values. It is practically easier to hold these
 /// mappings in one class since the conversion of control flow operations
 /// needs to look up block and function mappings.
 class ModuleTranslation {
 public:
   template <typename T = ModuleTranslation>
   static std::unique_ptr<llvm::Module>
   translateModule(Operation *m, llvm::LLVMContext &llvmContext,
-                  StringRef name = "LLVMDialectModule") {
+                  StringRef name = "LLVMDialectModule",
+                  llvm::Triple configTriple = llvm::Triple()) {
     if (!satisfiesLLVMModule(m))
       return nullptr;
     if (failed(checkSupportedModuleOps(m)))
       return nullptr;
     std::unique_ptr<llvm::Module> llvmModule =
-        prepareLLVMModule(m, llvmContext, name);
+        prepareLLVMModule(m, llvmContext, name, configTriple);
 
     LLVM::ensureDistinctSuccessors(m);
 
     T translator(m, std::move(llvmModule));
     if (failed(translator.convertFunctionSignatures()))
       return nullptr;
     if (failed(translator.convertGlobals()))
       return nullptr;
protected:
   virtual LogicalResult convertOmpParallel(Operation &op,
                                            llvm::IRBuilder<> &builder);
 
   /// Converts the type from MLIR LLVM dialect to LLVM.
   llvm::Type *convertType(LLVMType type);
 
   static std::unique_ptr<llvm::Module>
   prepareLLVMModule(Operation *m, llvm::LLVMContext &llvmContext,
-                    StringRef name);
+                    StringRef name, llvm::Triple configTriple);
 
   /// A helper to look up remapped operands in the value remapping table.
   SmallVector<llvm::Value *, 8> lookupValues(ValueRange values);
 
 private:
   /// Check whether the module contains only supported ops directly in its body.
   static LogicalResult checkSupportedModuleOps(Operation *m);
 

mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp

 
 #include "mlir/Target/LLVMIR.h"
 
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "mlir/Translation.h"
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/ToolOutputFile.h"
 
 using namespace mlir;
 
 std::unique_ptr<llvm::Module>
 mlir::translateModuleToLLVMIR(ModuleOp m, llvm::LLVMContext &llvmContext,
-                              StringRef name) {
+                              StringRef name, llvm::Triple triple) {
   auto llvmModule =
       LLVM::ModuleTranslation::translateModule<>(m, llvmContext, name);
   if (!llvmModule)
     emitError(m.getLoc(), "Fail to convert MLIR to LLVM IR");
   else if (verifyModule(*llvmModule))
     emitError(m.getLoc(), "LLVM IR fails to verify");
   return llvmModule;
 }

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

ModuleTranslation::lookupValues(ValueRange values) {
   for (Value v : values) {
     assert(valueMapping.count(v) && "referencing undefined value");
     remapped.push_back(valueMapping.lookup(v));
   }
   return remapped;
 }
 
 std::unique_ptr<llvm::Module> ModuleTranslation::prepareLLVMModule(
-    Operation *m, llvm::LLVMContext &llvmContext, StringRef name) {
+    Operation *m, llvm::LLVMContext &llvmContext, StringRef name,
+    llvm::Triple configTriple) {
   m->getContext()->getOrLoadDialect<LLVM::LLVMDialect>();
   auto llvmModule = std::make_unique<llvm::Module>(name, llvmContext);
   if (auto dataLayoutAttr =
           m->getAttr(LLVM::LLVMDialect::getDataLayoutAttrName()))
     llvmModule->setDataLayout(dataLayoutAttr.cast<StringAttr>().getValue());
 
   // Inject declarations for `malloc` and `free` functions that can be used in
   // memref allocation/deallocation coming from standard ops lowering.
   llvm::IRBuilder<> builder(llvmContext);
   llvmModule->getOrInsertFunction("malloc", builder.getInt8PtrTy(),
                                   builder.getInt64Ty());
   llvmModule->getOrInsertFunction("free", builder.getVoidTy(),
                                   builder.getInt8PtrTy());
 
+  if (configTriple.isKnownWindowsMSVCEnvironment()) {
+    // Dwarf debugging files will be generated by default, unless "CodeView" is
+    // set explicitly. Windows/MSVC should use CodeView instead.
+    llvmModule->addModuleFlag(llvm::Module::Warning, "CodeView", 1);
+  }
+
   return llvmModule;
 }