[MLIR][LLVM] Allow strings in module flag value

mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td

@@ -1332,18 +1332,21 @@ def ModuleFlagAttr
     Represents a single entry of llvm.module.flags metadata
     (llvm::Module::ModuleFlagEntry in LLVM). The first element is a behavior
     flag described by `ModFlagBehaviorAttr`, the second is a string ID
-    and third is the value of the flag (currently only integer constants
-    are supported).
+    and third is the value of the flag. Current supported types of values:
+      - Integer constants
+      - Strings
 
     Example:
     ```mlir
       #llvm.mlir.module_flag<error, "wchar_size", 4>
+      #llvm.mlir.module_flag<error, "probe-stack", "inline-asm">
     ```
   }];
   let parameters = (ins "ModFlagBehavior":$behavior,
                         "StringAttr":$key,
-                        "uint32_t":$value);
+                        "Attribute":$value);
   let assemblyFormat = "`<` $behavior `,` $key `,` $value `>`";
+  let genVerifyDecl = 1;
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp

@@ -375,3 +375,13 @@ TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
   return parentFunction.getOperation()->getAttrOfType<TargetFeaturesAttr>(
       getAttributeName());
 }
+
+LogicalResult
+ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
+                       LLVM::ModFlagBehavior flagBehavior, StringAttr key,
+                       Attribute value) {
+  if (!isa<IntegerAttr, StringAttr>(value))
+    return emitError()
+           << "only integer and string values are currently supported";
+  return success();
+}

mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp

@@ -16,6 +16,7 @@
 #include "mlir/Support/LLVM.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 
+#include "llvm/ADT/TypeSwitch.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/MDBuilder.h"
@@ -273,10 +274,25 @@ static void convertLinkerOptionsOp(ArrayAttr options,
 static void convertModuleFlagsOp(ArrayAttr flags, llvm::IRBuilderBase &builder,
                                  LLVM::ModuleTranslation &moduleTranslation) {
   llvm::Module *llvmModule = moduleTranslation.getLLVMModule();
-  for (auto flagAttr : flags.getAsRange<ModuleFlagAttr>())
+  for (auto flagAttr : flags.getAsRange<ModuleFlagAttr>()) {
+    llvm::Metadata *valueMetadata =
+        llvm::TypeSwitch<Attribute, llvm::Metadata *>(flagAttr.getValue())
+            .Case<StringAttr>([&](auto strAttr) {
+              return llvm::MDString::get(builder.getContext(),
+                                         strAttr.getValue());
+            })
+            .Case<IntegerAttr>([&](auto intAttr) {
+              return llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+                  llvm::Type::getInt32Ty(builder.getContext()),
+                  intAttr.getInt()));
+            })
+            .Default([](auto) { return nullptr; });
+
+    assert(valueMetadata && "expected valid metadata");
     llvmModule->addModuleFlag(
         convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
-        flagAttr.getKey().getValue(), flagAttr.getValue());
+        flagAttr.getKey().getValue(), valueMetadata);
+  }
 }
 
 static LogicalResult

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -525,18 +525,20 @@ LogicalResult ModuleImport::convertModuleFlagsMetadata() {
 
   SmallVector<Attribute> moduleFlags;
   for (const auto [behavior, key, val] : llvmModuleFlags) {
-    // Currently only supports most common: int constant values.
-    auto *constInt = llvm::mdconst::dyn_extract<llvm::ConstantInt>(val);
-    if (!constInt) {
+    Attribute valAttr = nullptr;
+    if (auto *constInt = llvm::mdconst::dyn_extract<llvm::ConstantInt>(val)) {
+      valAttr = builder.getI32IntegerAttr(constInt->getZExtValue());
+    } else if (auto *mdString = dyn_cast<llvm::MDString>(val)) {
+      valAttr = builder.getStringAttr(mdString->getString());
+    } else {
       emitWarning(mlirModule.getLoc())
-          << "unsupported module flag value: " << diagMD(val, llvmModule.get())
-          << ", only constant integer currently supported";
+          << "unsupported module flag value: " << diagMD(val, llvmModule.get());
       continue;
     }
 
     moduleFlags.push_back(builder.getAttr<ModuleFlagAttr>(
         convertModFlagBehaviorFromLLVM(behavior),
-        builder.getStringAttr(key->getString()), constInt->getZExtValue()));
+        builder.getStringAttr(key->getString()), valAttr));
   }
 
   if (!moduleFlags.empty())

mlir/test/Dialect/LLVMIR/invalid.mlir

@@ -1776,9 +1776,10 @@ llvm.mlir.alias external @y5 : i32 {
 // -----
 
 module {
-  // expected-error@+2 {{expected integer value}}
-  // expected-error@+1 {{failed to parse ModuleFlagAttr parameter 'value' which is to be a `uint32_t`}}
-  llvm.module_flags [#llvm.mlir.module_flag<error, "wchar_size", "yolo">]
+  llvm.func @foo()
+
+  // expected-error@below {{only integer and string values are currently supported}}
+  llvm.module_flags [#llvm.mlir.module_flag<error, "yolo", @foo>]
 }
 
 // -----

mlir/test/Dialect/LLVMIR/module-roundtrip.mlir

@@ -1,16 +1,18 @@
 // RUN: mlir-opt %s | mlir-opt | FileCheck %s
 
 module {
-  llvm.module_flags [#llvm.mlir.module_flag<error, "wchar_size", 4>,
-                     #llvm.mlir.module_flag<min, "PIC Level", 2>,
-                     #llvm.mlir.module_flag<max, "PIE Level", 2>,
-                     #llvm.mlir.module_flag<max, "uwtable", 2>,
-                     #llvm.mlir.module_flag<max, "frame-pointer", 1>]
+  llvm.module_flags [#llvm.mlir.module_flag<error, "wchar_size", 4 : i32>,
+                     #llvm.mlir.module_flag<min, "PIC Level", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "PIE Level", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "uwtable", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "frame-pointer", 1 : i32>,
+                     #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">]
 }
 
 // CHECK: llvm.module_flags [
-// CHECK-SAME: #llvm.mlir.module_flag<error, "wchar_size", 4>,
-// CHECK-SAME: #llvm.mlir.module_flag<min, "PIC Level", 2>,
-// CHECK-SAME: #llvm.mlir.module_flag<max, "PIE Level", 2>,
-// CHECK-SAME: #llvm.mlir.module_flag<max, "uwtable", 2>,
-// CHECK-SAME: #llvm.mlir.module_flag<max, "frame-pointer", 1>]
+// CHECK-SAME: #llvm.mlir.module_flag<error, "wchar_size", 4 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<min, "PIC Level", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<max, "PIE Level", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<max, "uwtable", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<max, "frame-pointer", 1 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">]

mlir/test/Target/LLVMIR/Import/module-flags.ll

@@ -1,25 +1,27 @@
 ; RUN: mlir-translate -import-llvm -split-input-file -verify-diagnostics %s | FileCheck %s
 
-!llvm.module.flags = !{!0, !1, !2, !3, !4}
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5}
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{i32 8, !"PIC Level", i32 2}
 !2 = !{i32 7, !"PIE Level", i32 2}
 !3 = !{i32 7, !"uwtable", i32 2}
 !4 = !{i32 7, !"frame-pointer", i32 1}
+!5 = !{i32 4, !"probe-stack", !"inline-asm"}
 
 ; CHECK-LABEL: module attributes {{.*}} {
 ; CHECK: llvm.module_flags [
-; CHECK-SAME: #llvm.mlir.module_flag<error, "wchar_size", 4>,
-; CHECK-SAME: #llvm.mlir.module_flag<min, "PIC Level", 2>,
-; CHECK-SAME: #llvm.mlir.module_flag<max, "PIE Level", 2>,
-; CHECK-SAME: #llvm.mlir.module_flag<max, "uwtable", 2>,
-; CHECK-SAME: #llvm.mlir.module_flag<max, "frame-pointer", 1>]
-; CHECK: }
+; CHECK-SAME: #llvm.mlir.module_flag<error, "wchar_size", 4 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<min, "PIC Level", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<max, "PIE Level", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<max, "uwtable", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<max, "frame-pointer", 1 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">]
 
 ; // -----
-
-!llvm.module.flags = !{!0}
-
-; expected-warning@-5{{unsupported module flag value: !"yolo_more", only constant integer currently supported}}
-!0 = !{i32 1, !"yolo", !"yolo_more"}
+; expected-warning@-2 {{unsupported module flag value: !4 = !{!"foo", i32 1}}}
+!10 = !{ i32 1, !"foo", i32 1 }
+!11 = !{ i32 4, !"bar", i32 37 }
+!12 = !{ i32 2, !"qux", i32 42 }
+!13 = !{ i32 3, !"qux", !{ !"foo", i32 1 }}
+!llvm.module.flags = !{ !10, !11, !12, !13 }

mlir/test/Target/LLVMIR/llvmir.mlir

@@ -2809,11 +2809,11 @@ llvm.func @call_intrin_with_opbundle(%arg0 : !llvm.ptr) {
 // -----
 
 module {
-  llvm.module_flags [#llvm.mlir.module_flag<error, "wchar_size", 4>,
-                     #llvm.mlir.module_flag<min, "PIC Level", 2>,
-                     #llvm.mlir.module_flag<max, "PIE Level", 2>,
-                     #llvm.mlir.module_flag<max, "uwtable", 2>,
-                     #llvm.mlir.module_flag<max, "frame-pointer", 1>]
+  llvm.module_flags [#llvm.mlir.module_flag<error, "wchar_size", 4 : i32>,
+                     #llvm.mlir.module_flag<min, "PIC Level", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "PIE Level", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "uwtable", 2 : i32>,
+                     #llvm.mlir.module_flag<max, "frame-pointer", 1 : i32>]
 }
 
 // CHECK: !llvm.module.flags = !{![[#WCHAR:]], ![[#PIC:]], ![[#PIE:]], ![[#UWTABLE:]], ![[#FrameP:]], ![[#DBG:]]}
@@ -2830,7 +2830,7 @@ module {
 // Verifies that the debug info version is not added twice, if it's already present initially.
 
 module {
-  llvm.module_flags [#llvm.mlir.module_flag<warning, "Debug Info Version", 3>]
+  llvm.module_flags [#llvm.mlir.module_flag<warning, "Debug Info Version", 3 : i32>]
 }
 
 // CHECK: !llvm.module.flags = !{![[#DBG:]]}