-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[MLIR][LLVMIR] Add module flags support #130679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MLIR][LLVMIR] Add module flags support #130679
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-llvm Author: Bruno Cardoso Lopes (bcardosolopes) ChangesImport and translation support. Note that existing support (prior to this PR) already covers enough in translation specifically to emit "Debug Info Version". Also, the debug info version metadata is being emitted even though the imported IR has no information and is showing up in some tests (will fix that in another PR). Full diff: https://github.com/llvm/llvm-project/pull/130679.diff 13 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 267389774bd5a..760ed55897cd6 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -1267,4 +1267,30 @@ def WorkgroupAttributionAttr
let assemblyFormat = "`<` $num_elements `,` $element_type `>`";
}
+//===----------------------------------------------------------------------===//
+// ModuleFlagAttr
+//===----------------------------------------------------------------------===//
+
+def ModuleFlagAttr
+ : LLVM_Attr<"ModuleFlag", "mlir.module_flag"> {
+ let summary = "LLVM module flag metadata";
+ let description = [{
+ 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 constant
+ supported).
+
+ Example:
+ ```mlir
+ #llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>
+ ]
+ ```
+ }];
+ let parameters = (ins "ModFlagBehaviorAttr":$behavior,
+ "StringAttr":$key,
+ "IntegerAttr":$value);
+ let assemblyFormat = "`<` $behavior `,` $key `,` $value `>`";
+}
+
#endif // LLVMIR_ATTRDEFS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
index d35f48b13b2d8..46fae44f7b0fa 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
@@ -33,6 +33,7 @@ def LLVM_Dialect : Dialect {
static StringRef getAliasScopesAttrName() { return "alias_scopes"; }
static StringRef getAccessGroupsAttrName() { return "access_groups"; }
static StringRef getIdentAttrName() { return "llvm.ident"; }
+ static StringRef getModuleFlags() { return "llvm.module.flags"; }
static StringRef getCommandlineAttrName() { return "llvm.commandline"; }
/// Names of llvm parameter attributes.
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index c08b75de03647..a9de787806452 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -818,4 +818,37 @@ def FPExceptionBehaviorAttr : LLVM_EnumAttr<
let cppNamespace = "::mlir::LLVM";
}
+//===----------------------------------------------------------------------===//
+// Module Flag Behavior
+//===----------------------------------------------------------------------===//
+
+// These values must match llvm::Module::ModFlagBehavior ones.
+// See llvm/include/llvm/IR/Module.h.
+def ModFlagBehaviorError
+ : LLVM_EnumAttrCase<"Error", "error", "Error", 1>;
+def ModFlagBehaviorWarning
+ : LLVM_EnumAttrCase<"Warning", "warning", "Warning", 2>;
+def ModFlagBehaviorRequire
+ : LLVM_EnumAttrCase<"Require", "require", "Require", 3>;
+def ModFlagBehaviorOverride
+ : LLVM_EnumAttrCase<"Override", "override", "Override", 4>;
+def ModFlagBehaviorAppend
+ : LLVM_EnumAttrCase<"Append", "append", "Append", 5>;
+def ModFlagBehaviorAppendUnique
+ : LLVM_EnumAttrCase<"AppendUnique", "append_unique", "AppendUnique", 6>;
+def ModFlagBehaviorMax
+ : LLVM_EnumAttrCase<"Max", "max", "Max", 7>;
+def ModFlagBehaviorMin
+ : LLVM_EnumAttrCase<"Min", "min", "Min", 8>;
+
+def ModFlagBehaviorAttr : LLVM_EnumAttr<
+ "ModFlagBehavior",
+ "::llvm::Module::ModFlagBehavior",
+ "LLVM Module Flag Behavior",
+ [ModFlagBehaviorError, ModFlagBehaviorWarning, ModFlagBehaviorRequire,
+ ModFlagBehaviorOverride, ModFlagBehaviorAppend,
+ ModFlagBehaviorAppendUnique, ModFlagBehaviorMax, ModFlagBehaviorMin]> {
+ let cppNamespace = "::mlir::LLVM";
+}
+
#endif // LLVMIR_ENUMS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 121cf53393f15..db6e3faf710c1 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -2155,4 +2155,36 @@ def LLVM_LinkerOptionsOp
let hasVerifier = 1;
}
+//===--------------------------------------------------------------------===//
+// ModuleFlagsOp
+//===--------------------------------------------------------------------===//
+
+def LLVM_ModuleFlagsOp
+ : LLVM_Op<"module_flags"> {
+ let summary = "Information about module properties";
+ let description = [{
+ Represents the equivalent in MLIR for LLVM's `llvm.module.flags` metadata,
+ which requires a list of metadata triplets. Each triplet entry is described
+ by a `ModuleFlagAttr`.
+
+ Example:
+ ```mlir
+ llvm.module.flags [
+ #llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>,
+ #llvm.mlir.module_flag<8 : i64, "PIC Level", 2 : i32>
+ ]
+ ```
+ }];
+ let arguments = (ins ArrayAttr:$flags);
+ let assemblyFormat = [{
+ $flags attr-dict
+ }];
+
+ let llvmBuilder = [{
+ convertModuleFlagsOp($flags, builder, moduleTranslation);
+ }];
+
+ let hasVerifier = 1;
+}
+
#endif // LLVMIR_OPS
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index c0af924e0aecd..cf83c6c118d6e 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -218,6 +218,10 @@ class ModuleImport {
/// LLVM dialect operation.
LogicalResult convertLinkerOptionsMetadata();
+ /// Converts !llvm.module.flags metadata to the XYZ
+ /// LLVM dialect operation.
+ LogicalResult convertModuleFlagsMetadata();
+
/// Converts !llvm.ident metadata to the llvm.ident LLVM ModuleOp attribute.
LogicalResult convertIdentMetadata();
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 8a6325af201f4..1c28e6ca5fa7c 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3711,6 +3711,21 @@ LogicalResult LinkerOptionsOp::verify() {
return success();
}
+//===----------------------------------------------------------------------===//
+// ModuleFlagsOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult ModuleFlagsOp::verify() {
+ if (mlir::Operation *parentOp = (*this)->getParentOp();
+ parentOp && !satisfiesLLVMModule(parentOp))
+ return emitOpError("must appear at the module level");
+ for (auto &flag : getFlags()) {
+ if (!isa<ModuleFlagAttr>(flag))
+ return emitOpError("expected a module flag attribute");
+ }
+ return success();
+}
+
//===----------------------------------------------------------------------===//
// InlineAsmOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
index 25599efe64322..c1df6fdd9ef34 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
@@ -270,6 +270,19 @@ static void convertLinkerOptionsOp(ArrayAttr options,
linkerMDNode->addOperand(listMDNode);
}
+static void convertModuleFlagsOp(ArrayAttr flags, llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation) {
+ llvm::Module *llvmModule = moduleTranslation.getLLVMModule();
+ for (Attribute attr : flags) {
+ auto flag = cast<ModuleFlagAttr>(attr);
+ auto intVal = dyn_cast<IntegerAttr>(flag.getValue());
+ assert(intVal && "expected integer attribute");
+ llvmModule->addModuleFlag(
+ (llvm::Module::ModFlagBehavior)flag.getBehavior().getValue(),
+ flag.getKey().getValue(), (uint32_t)intVal.getUInt());
+ }
+}
+
static LogicalResult
convertOperationImpl(Operation &opInst, llvm::IRBuilderBase &builder,
LLVM::ModuleTranslation &moduleTranslation) {
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index f24c2777cbbb8..728a58c28804d 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -517,6 +517,45 @@ void ModuleImport::addDebugIntrinsic(llvm::CallInst *intrinsic) {
debugIntrinsics.insert(intrinsic);
}
+LogicalResult ModuleImport::convertModuleFlagsMetadata() {
+ SmallVector<llvm::Module::ModuleFlagEntry, 4> llvmModuleFlags;
+ for (const llvm::NamedMDNode &named : llvmModule->named_metadata()) {
+ if (named.getName() != LLVMDialect::getModuleFlags())
+ continue;
+ llvmModule->getModuleFlagsMetadata(llvmModuleFlags);
+ break; // there can only be one module flags.
+ }
+
+ SmallVector<Attribute, 4> 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) {
+ return emitWarning(mlirModule.getLoc())
+ << "unsupported module flag value: "
+ << diagMD(val, llvmModule.get())
+ << ", only constant integer currently supported";
+ }
+ auto valAttr = builder.getIntegerAttr(
+ IntegerType::get(context, constInt->getType()->getIntegerBitWidth()),
+ constInt->getValue());
+
+ ModFlagBehaviorAttr behaviorAttr = ModFlagBehaviorAttr::get(
+ builder.getContext(), (ModFlagBehavior)behavior);
+
+ moduleFlags.push_back(
+ ModuleFlagAttr::get(builder.getContext(), behaviorAttr,
+ builder.getStringAttr(key->getString()), valAttr));
+ }
+
+ if (!moduleFlags.empty()) {
+ builder.create<LLVM::ModuleFlagsOp>(mlirModule.getLoc(),
+ builder.getArrayAttr(moduleFlags));
+ }
+
+ return success();
+}
+
LogicalResult ModuleImport::convertLinkerOptionsMetadata() {
for (const llvm::NamedMDNode &named : llvmModule->named_metadata()) {
if (named.getName() != "llvm.linker.options")
@@ -596,6 +635,8 @@ LogicalResult ModuleImport::convertMetadata() {
}
if (failed(convertLinkerOptionsMetadata()))
return failure();
+ if (failed(convertModuleFlagsMetadata()))
+ return failure();
if (failed(convertIdentMetadata()))
return failure();
if (failed(convertCommandlineMetadata()))
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index fcb6ae07f4912..69c798f5e926e 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1764,3 +1764,10 @@ llvm.mlir.alias external @y5 : i32 {
llvm.return %0 : !llvm.ptr<4>
}
+// -----
+
+module {
+ // expected-error@+2 {{invalid kind of attribute specified}}
+ // expected-error@+1 {{failed to parse ModuleFlagAttr parameter 'value' which is to be a `IntegerAttr`}}
+ llvm.module_flags [#llvm.mlir.module_flag<1 : i64, "wchar_size", "yolo">]
+}
diff --git a/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir b/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir
new file mode 100644
index 0000000000000..55b79437f1c5c
--- /dev/null
+++ b/mlir/test/Dialect/LLVMIR/module-roundtrip.mlir
@@ -0,0 +1,16 @@
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+
+module {
+ llvm.module_flags [#llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>,
+ #llvm.mlir.module_flag<8 : i64, "PIC Level", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "PIE Level", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "uwtable", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "frame-pointer", 1 : i32>]
+}
+
+// CHECK: llvm.module_flags [
+// CHECK-SAME: #llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<8 : i64, "PIC Level", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "PIE Level", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "uwtable", 2 : i32>,
+// CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "frame-pointer", 1 : i32>]
diff --git a/mlir/test/Target/LLVMIR/Import/module-flags.ll b/mlir/test/Target/LLVMIR/Import/module-flags.ll
new file mode 100644
index 0000000000000..feff48836bba7
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/module-flags.ll
@@ -0,0 +1,25 @@
+; RUN: mlir-translate -import-llvm -split-input-file -verify-diagnostics %s | FileCheck %s
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4}
+
+!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}
+
+; CHECK-LABEL: module attributes {{.*}} {
+; CHECK: llvm.module_flags [
+; CHECK-SAME: #llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<8 : i64, "PIC Level", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "PIE Level", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "uwtable", 2 : i32>,
+; CHECK-SAME: #llvm.mlir.module_flag<7 : i64, "frame-pointer", 1 : i32>]
+; CHECK: }
+
+; // -----
+
+!llvm.module.flags = !{!0}
+
+; expected-warning@-5{{unsupported module flag value: !"yolo_more", only constant integer currently supported}}
+!0 = !{i32 1, !"yolo", !"yolo_more"}
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index 15658ea606812..b973d0087ad68 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -348,6 +348,20 @@ llvm.func @foo() {
// -----
+llvm.func @foo() {
+ // expected-error @below{{must appear at the module level}}
+ llvm.module_flags [#llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>]
+}
+
+// -----
+
+module attributes {} {
+ // expected-error @below{{expected a module flag attribute}}
+ llvm.module_flags [4 : i32]
+}
+
+// -----
+
module @does_not_exist {
// expected-error @below{{resource does not exist}}
llvm.mlir.global internal constant @constant(dense_resource<test0> : tensor<4xf32>) : !llvm.array<4 x f32>
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index db2e08742dbca..2f2356a987b33 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -2486,6 +2486,25 @@ llvm.linker_options ["/DEFAULTLIB:", "libcmtd"]
// -----
+module {
+ llvm.module_flags [#llvm.mlir.module_flag<1 : i64, "wchar_size", 4 : i32>,
+ #llvm.mlir.module_flag<8 : i64, "PIC Level", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "PIE Level", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "uwtable", 2 : i32>,
+ #llvm.mlir.module_flag<7 : i64, "frame-pointer", 1 : i32>]
+}
+
+// CHECK: !llvm.module.flags = !{![[DBG:.*]], ![[WCHAR:.*]], ![[PIC:.*]], ![[PIE:.*]], ![[UWTABLE:.*]], ![[FP:.*]]}
+
+// CHECK: ![[DBG]] = !{i32 2, !"Debug Info Version", i32 3}
+// CHECK: ![[WCHAR]] = !{i32 1, !"wchar_size", i32 4}
+// CHECK: ![[PIC]] = !{i32 8, !"PIC Level", i32 2}
+// CHECK: ![[PIE]] = !{i32 7, !"PIE Level", i32 2}
+// CHECK: ![[UWTABLE]] = !{i32 7, !"uwtable", i32 2}
+// CHECK: ![[FP]] = !{i32 7, !"frame-pointer", i32 1}
+
+// -----
+
// CHECK: @big_ = common global [4294967296 x i8] zeroinitializer
llvm.mlir.global common @big_(dense<0> : vector<4294967296xi8>) {addr_space = 0 : i32} : !llvm.array<4294967296 x i8>
|
gysit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a first pass an I wonder if we need the operation. Couldn't we just add these attributes to the module? That would possibly simplify the PR quite a bit.
mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
Outdated
Show resolved
Hide resolved
mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
Outdated
Show resolved
Hide resolved
mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
Outdated
Show resolved
Hide resolved
We could, I did that initially but then decided to go the |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
I see. Yeah let's stick with the current approach then and strive for "consistency" |
|
Applied all reviews, thanks! |
gysit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes.
LGTM modulo last nits.
Import and translation support. Note that existing support (prior to this PR) already covers enough in translation specifically to emit "Debug Info Version". Also, the debug info version metadata is being emitted even though the imported IR has no information and is showing up in some tests (will fix that in another PR).
Co-authored-by: Tobias Gysi <[email protected]>
Co-authored-by: Tobias Gysi <[email protected]>
Co-authored-by: Tobias Gysi <[email protected]>
Co-authored-by: Tobias Gysi <[email protected]>
Co-authored-by: Tobias Gysi <[email protected]>
Co-authored-by: Henrich Lauko <[email protected]>
Co-authored-by: Henrich Lauko <[email protected]>
Co-authored-by: Henrich Lauko <[email protected]>
289c62f to
220fa4a
Compare
Import and translation support.
Note that existing support (prior to this PR) already covers enough in translation specifically to emit "Debug Info Version". Also, the debug info version metadata is being emitted even though the imported IR has no information and is showing up in some tests (will fix that in another PR).