[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 ↗

(On Diff #304878)

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/Dialect/LLVMIR/LLVMOpBase.td

def LLVM_Dialect : Dialect {
   let extraClassDeclaration = [{
     /// Name of the data layout attributes.
     static StringRef getDataLayoutAttrName() { return "llvm.data_layout"; }
 
     /// Verifies if the given string is a well-formed data layout descriptor.
     /// Uses `reportError` to report errors.
     static LogicalResult verifyDataLayoutString(
         StringRef descr, llvm::function_ref<void (const Twine &)> reportError);
+
+    /// Name of the target triple attribute.
+    static StringRef getTargetTripleAttrName() { return "llvm.target_triple"; }
   }];
 }
 
 //===----------------------------------------------------------------------===//
 // LLVM dialect type constraints.
 //===----------------------------------------------------------------------===//
 
 // LLVM dialect type.

mlir/lib/Target/LLVMIR/DebugTranslation.cpp

compileUnit = builder.createCompileUnit(
       builder.createFile(llvmModule.getModuleIdentifier(), "/"),
       /*Producer=*/"mlir", /*isOptimized=*/true, /*Flags=*/"", /*RV=*/0);
 
   // Mark this module as having debug information.
   StringRef debugVersionKey = "Debug Info Version";
   if (!llvmModule.getModuleFlag(debugVersionKey))
     llvmModule.addModuleFlag(llvm::Module::Warning, debugVersionKey,
                              llvm::DEBUG_METADATA_VERSION);
+
+  if (auto targetTripleAttr =
+          module->getAttr(LLVM::LLVMDialect::getTargetTripleAttrName())) {
+    auto targetTriple =
+        llvm::Triple(targetTripleAttr.cast<StringAttr>().getValue());
+    if (targetTriple.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);
+    }
+  }
 }
 
 /// Finalize the translation of debug information.
 void DebugTranslation::finalize() { builder.finalize(); }
 
 /// Attempt to extract a filename for the given loc.
 static FileLineColLoc extractFileLoc(Location loc) {
   if (auto fileLoc = loc.dyn_cast<FileLineColLoc>())

mlir/test/Target/llvmir.mlir

 
 // -----
 
 // CHECK: target datalayout = "E"
 module attributes {llvm.data_layout = "E"} {
   llvm.func @module_big_endian()
 }
 
+// -----
+
+// CHECK: "CodeView", i32 1
+module attributes {llvm.target_triple = "x86_64-pc-windows-msvc"} {}
+
+// -----
+
+// CHECK-NOT: "CodeView", i32 1
+module attributes {llvm.target_triple = "aarch64-linux-android"} {}
+
+// -----
+
+// CHECK-NOT: "CodeView", i32 1
+module attributes {} {}