[Mlir-commits] [mlir] [MLIR][LLVM] Allow strings in module flag value (PR #136793)
Bruno Cardoso Lopes
llvmlistbot at llvm.org
Wed Apr 23 15:01:09 PDT 2025
https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/136793
>From 5d2743beb2630adb5f2b0bfd12ce5c432484b6e2 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Tue, 22 Apr 2025 16:11:23 -0700
Subject: [PATCH 1/2] [MLIR][LLVM] Allow strings in module flag value
Expand support beyond integers.
Next step is to support more complex metadata values (e.g. !"CG Profile"
and !"ProfileSummary"), but that's a bit more complex and deserves it own PR.
---
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 9 ++++---
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 10 +++++++
.../LLVMIR/LLVMToLLVMIRTranslation.cpp | 16 +++++++++---
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 18 ++++++++-----
mlir/test/Dialect/LLVMIR/invalid.mlir | 7 ++---
.../test/Dialect/LLVMIR/module-roundtrip.mlir | 22 +++++++++-------
.../test/Target/LLVMIR/Import/module-flags.ll | 26 ++++++++++---------
7 files changed, 69 insertions(+), 39 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 0f195ff82c3ff..f53f95ee9ba49 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/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;
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index e4f9d6f987401..8ceb470a3d5b1 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/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,
+ mlir::LLVM::ModFlagBehavior flagBehavior,
+ mlir::StringAttr key, mlir::Attribute value) {
+ if (!isa<mlir::IntegerAttr, mlir::StringAttr>(value))
+ return emitError()
+ << "only integer and string values are currently supported";
+ return success();
+}
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
index 7038b5d73d266..a348a9699b06c 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
@@ -273,10 +273,18 @@ 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>())
- llvmModule->addModuleFlag(
- convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
- flagAttr.getKey().getValue(), flagAttr.getValue());
+ for (auto flagAttr : flags.getAsRange<ModuleFlagAttr>()) {
+ if (auto intAttr = dyn_cast<mlir::IntegerAttr>(flagAttr.getValue()))
+ llvmModule->addModuleFlag(
+ convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
+ flagAttr.getKey().getValue(), intAttr.getUInt());
+ else if (auto strAttr = dyn_cast<mlir::StringAttr>(flagAttr.getValue())) {
+ llvmModule->addModuleFlag(
+ convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
+ flagAttr.getKey().getValue(),
+ llvm::MDString::get(builder.getContext(), strAttr.getValue()));
+ }
+ }
}
static LogicalResult
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index df7c8d6ea3579..4e40906e74882 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -525,18 +525,22 @@ 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";
- continue;
+ << "unsupported module flag value: " << diagMD(val, llvmModule.get());
}
+ if (!valAttr)
+ continue;
+
moduleFlags.push_back(builder.getAttr<ModuleFlagAttr>(
convertModFlagBehaviorFromLLVM(behavior),
- builder.getStringAttr(key->getString()), constInt->getZExtValue()));
+ builder.getStringAttr(key->getString()), valAttr));
}
if (!moduleFlags.empty())
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 0cd6b1f20a1bf..15bcbef770e96 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1776,9 +1776,10 @@ llvm.mlir.alias external @y5 : i32 {
// -----
module {
- // expected-error at +2 {{expected integer value}}
- // expected-error at +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 at +1 {{only integer and string values are currently supported}}
+ llvm.module_flags [#llvm.mlir.module_flag<error, "yolo", @foo>]
}
// -----
diff --git a/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir b/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir
index d99a93c1e8565..a94514da9818f 100644
--- a/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir
+++ b/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">]
diff --git a/mlir/test/Target/LLVMIR/Import/module-flags.ll b/mlir/test/Target/LLVMIR/Import/module-flags.ll
index b7b686f94c7f4..a0b332c0e41c3 100644
--- a/mlir/test/Target/LLVMIR/Import/module-flags.ll
+++ b/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 at -5{{unsupported module flag value: !"yolo_more", only constant integer currently supported}}
-!0 = !{i32 1, !"yolo", !"yolo_more"}
+; expected-warning at -2{{unsupported module flag value: !4 = !{!"foo", i32 1}}}
+!0 = !{ i32 1, !"foo", i32 1 }
+!1 = !{ i32 4, !"bar", i32 37 }
+!2 = !{ i32 2, !"qux", i32 42 }
+!3 = !{ i32 3, !"qux", !{ !"foo", i32 1 }}
+!llvm.module.flags = !{ !0, !1, !2, !3 }
>From 7e4aeb439f700767acdbf5d2299cc97c3571633f Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Wed, 23 Apr 2025 14:47:02 -0700
Subject: [PATCH 2/2] address reviews
---
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 6 +++---
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 4 +---
mlir/test/Dialect/LLVMIR/invalid.mlir | 2 +-
mlir/test/Target/LLVMIR/Import/module-flags.ll | 12 ++++++------
4 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 8ceb470a3d5b1..f3ebb8a565ea4 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -378,9 +378,9 @@ TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
LogicalResult
ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
- mlir::LLVM::ModFlagBehavior flagBehavior,
- mlir::StringAttr key, mlir::Attribute value) {
- if (!isa<mlir::IntegerAttr, mlir::StringAttr>(value))
+ LLVM::ModFlagBehavior flagBehavior, StringAttr key,
+ Attribute value) {
+ if (!isa<IntegerAttr, StringAttr>(value))
return emitError()
<< "only integer and string values are currently supported";
return success();
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 4e40906e74882..3f80002c15ebb 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -533,10 +533,8 @@ LogicalResult ModuleImport::convertModuleFlagsMetadata() {
} else {
emitWarning(mlirModule.getLoc())
<< "unsupported module flag value: " << diagMD(val, llvmModule.get());
- }
-
- if (!valAttr)
continue;
+ }
moduleFlags.push_back(builder.getAttr<ModuleFlagAttr>(
convertModFlagBehaviorFromLLVM(behavior),
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 15bcbef770e96..a3cd9572933ae 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1778,7 +1778,7 @@ llvm.mlir.alias external @y5 : i32 {
module {
llvm.func @foo()
- // expected-error at +1 {{only integer and string values are currently supported}}
+ // expected-error at below {{only integer and string values are currently supported}}
llvm.module_flags [#llvm.mlir.module_flag<error, "yolo", @foo>]
}
diff --git a/mlir/test/Target/LLVMIR/Import/module-flags.ll b/mlir/test/Target/LLVMIR/Import/module-flags.ll
index a0b332c0e41c3..e6bb2c0ffb32d 100644
--- a/mlir/test/Target/LLVMIR/Import/module-flags.ll
+++ b/mlir/test/Target/LLVMIR/Import/module-flags.ll
@@ -19,9 +19,9 @@
; CHECK-SAME: #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">]
; // -----
-; expected-warning at -2{{unsupported module flag value: !4 = !{!"foo", i32 1}}}
-!0 = !{ i32 1, !"foo", i32 1 }
-!1 = !{ i32 4, !"bar", i32 37 }
-!2 = !{ i32 2, !"qux", i32 42 }
-!3 = !{ i32 3, !"qux", !{ !"foo", i32 1 }}
-!llvm.module.flags = !{ !0, !1, !2, !3 }
+; expected-warning at -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 }
More information about the Mlir-commits
mailing list