[Mlir-commits] [mlir] [MLIR][LLVMIR] Handle MDTuple-of-MDStrings module flags (e.g. riscv-isa) (PR #188741)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

The "riscv-isa" LLVM module flag stores its value as an MDTuple containing MDStrings (e.g. `\!{\!"rv64i2p1", \!"m2p0"}`). Previously, this fell through the unrecognized-key path in `convertModuleFlagValueFromMDTuple`, which emitted a warning and dropped the flag during import.

This patch adds generic handling for MDTuples whose operands are all MDStrings:
- Import: convert to `ArrayAttr<StringAttr>` in `convertModuleFlagValueFromMDTuple`
- Export: convert `ArrayAttr<StringAttr>` back to an MDTuple of MDStrings in `convertModuleFlagValue`, enabling a lossless round-trip
- Verifier: allow `ArrayAttr<StringAttr>` as a valid `ModuleFlagAttr` value for keys not otherwise handled by specific verifier branches

Fixes #<!-- -->188122

Assisted-by: Claude Code

---
Full diff: https://github.com/llvm/llvm-project/pull/188741.diff


5 Files Affected:

- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp (+6) 
- (modified) mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp (+7) 
- (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+11-1) 
- (modified) mlir/test/Target/LLVMIR/Import/module-flags.ll (+12) 
- (modified) mlir/test/Target/LLVMIR/llvmir.mlir (+11) 


``````````diff
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 9f87e502a0bf1..1b3c4933932e1 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -531,6 +531,12 @@ ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
   if (isa<IntegerAttr, StringAttr>(value))
     return success();
 
+  // Allow ArrayAttr of StringAttrs to represent MDTuples of MDStrings
+  // (e.g. the "riscv-isa" module flag).
+  if (auto arrayAttr = dyn_cast<ArrayAttr>(value))
+    if (llvm::all_of(arrayAttr, [](Attribute a) { return isa<StringAttr>(a); }))
+      return success();
+
   return emitError() << "only integer and string values are currently "
                         "supported for unknown key '"
                      << key << "'";
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
index 8c3680033b2b9..3f1f881159642 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
@@ -311,6 +311,13 @@ convertModuleFlagValue(StringRef key, ArrayAttr arrayAttr,
     }
     return llvm::MDTuple::getDistinct(context, nodes);
   }
+  // Handle ArrayAttr of StringAttrs (e.g. "riscv-isa") by converting back to
+  // an MDTuple of MDStrings for a lossless round-trip.
+  if (llvm::all_of(arrayAttr, [](Attribute a) { return isa<StringAttr>(a); })) {
+    for (StringAttr strAttr : arrayAttr.getAsRange<StringAttr>())
+      nodes.push_back(llvm::MDString::get(context, strAttr.getValue()));
+    return llvm::MDTuple::get(context, nodes);
+  }
   return nullptr;
 }
 
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 25aaccecc56a2..b5d8a174f9df9 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -841,7 +841,17 @@ convertModuleFlagValueFromMDTuple(ModuleOp mlirModule,
   if (key == LLVMDialect::getModuleFlagKeyProfileSummaryName())
     return convertProfileSummaryModuleFlagValue(mlirModule, llvmModule,
                                                 mdTuple);
-  return nullptr;
+  // Handle MDTuples whose operands are all MDStrings (e.g. "riscv-isa").
+  // Convert them to ArrayAttr of StringAttrs for a lossless round-trip.
+  Builder builder(mlirModule->getContext());
+  SmallVector<Attribute> strings;
+  for (const llvm::MDOperand &operand : mdTuple->operands()) {
+    auto *mdString = dyn_cast<llvm::MDString>(operand);
+    if (!mdString)
+      return nullptr;
+    strings.push_back(builder.getStringAttr(mdString->getString()));
+  }
+  return builder.getArrayAttr(strings);
 }
 
 LogicalResult ModuleImport::convertModuleFlagsMetadata() {
diff --git a/mlir/test/Target/LLVMIR/Import/module-flags.ll b/mlir/test/Target/LLVMIR/Import/module-flags.ll
index 725bd14deb651..e406bf985ff90 100644
--- a/mlir/test/Target/LLVMIR/Import/module-flags.ll
+++ b/mlir/test/Target/LLVMIR/Import/module-flags.ll
@@ -95,3 +95,15 @@ declare void @to()
 ; CHECK-SAME: <cut_off = 10000, min_count = 86427, num_counts = 1>,
 ; CHECK-SAME: <cut_off = 100000, min_count = 86427, num_counts = 1>
 ; CHECK-SAME: >>]
+
+; // -----
+
+; Test that MDTuples of MDStrings (e.g. "riscv-isa") are imported as ArrayAttr
+; of StringAttrs rather than emitting a warning and dropping the flag.
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"riscv-isa", !1}
+!1 = !{!"rv64i2p1", !"m2p0"}
+
+; CHECK: llvm.module_flags [#llvm.mlir.module_flag<error, "riscv-isa", ["rv64i2p1", "m2p0"]>]
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 4ac3776d87c8e..f97c5aa983e26 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -3366,6 +3366,17 @@ llvm.module_flags [#llvm.mlir.module_flag<error, "ProfileSummary",
 
 // -----
 
+// Test that ArrayAttr of StringAttrs (e.g. "riscv-isa") is exported as an
+// MDTuple of MDStrings for a lossless round-trip.
+
+llvm.module_flags [#llvm.mlir.module_flag<error, "riscv-isa", ["rv64i2p1", "m2p0"]>]
+
+// CHECK: !llvm.module.flags = !{![[#RISCV:]], {{.*}}}
+// CHECK: ![[#RISCV]] = !{i32 1, !"riscv-isa", ![[#ISA:]]}
+// CHECK: ![[#ISA]] = !{!"rv64i2p1", !"m2p0"}
+
+// -----
+
 module attributes {llvm.dependent_libraries = ["foo", "bar"]} {}
 
 // CHECK: !llvm.dependent-libraries =  !{![[#LIBFOO:]], ![[#LIBBAR:]]}

``````````

</details>


https://github.com/llvm/llvm-project/pull/188741