[Mlir-commits] [mlir] [mlir][LLVMIR] Bound import diagnostics and share one slot tracker (PR #229956)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Oct 7 17:00:36 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-llvm
Author: Bruno Cardoso Lopes (bcardosolopes)
<details>
<summary>Changes</summary>
Import diagnostics render LLVM entities unbounded: a global prints its entire initializer (a merged vtable reaches megabytes), and every rendering builds its own ModuleSlotTracker, whose module walk makes a per-instruction diagnostic quadratic in module size. To the scale we link and use the LLVM IR dialect, this has bitten us many times downstream, usually leading to OOM's and other silly behaviors from something that isn't supported anyways. This solution has helped us finding and fixing a lot of missing attributes, which will be contributed soon.
Cap each rendering at 256 characters and print globals by name only, diagnostics only need to identify the entity. Share one lazily created tracker across the import instead: printing through it numbers exactly like the per-call tracker, the module is never mutated during import so slots stay valid, and each print establishes its own function context. Thread the tracker through the module-flag converters so every diagnostic uses the same numbering.
Assisted-by: Muse Code (Muse).
---
Patch is 35.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/229956.diff
5 Files Affected:
- (modified) mlir/include/mlir/Target/LLVMIR/ModuleImport.h (+16)
- (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+153-96)
- (modified) mlir/test/Target/LLVMIR/Import/function-metadata.ll (+2-2)
- (modified) mlir/test/Target/LLVMIR/Import/global-variables.ll (+7)
- (modified) mlir/test/Target/LLVMIR/Import/import-failure.ll (+34)
``````````diff
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 3bb024a88f283c..ab46245b040d09 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -24,6 +24,7 @@
namespace llvm {
class BasicBlock;
+class ModuleSlotTracker;
class CallBase;
class DbgVariableIntrinsic;
class Function;
@@ -51,6 +52,7 @@ class ModuleImport {
ModuleImport(ModuleOp mlirModule, std::unique_ptr<llvm::Module> llvmModule,
bool emitExpensiveWarnings, bool importEmptyDICompositeTypes,
bool preferUnregisteredIntrinsics, bool importStructsAsLiterals);
+ ~ModuleImport();
/// Calls the LLVMImportInterface initialization that queries the registered
/// dialect interfaces for the supported LLVM IR intrinsics and metadata kinds
@@ -315,6 +317,13 @@ class ModuleImport {
}
private:
+ /// Renders `value` into a bounded string for a diagnostic.
+ std::string renderValue(const llvm::Value &value);
+ /// Renders `node` into a bounded string for a diagnostic.
+ std::string renderMetadata(const llvm::Metadata *node);
+ /// Shared slot tracker for diagnostics, created on first use.
+ llvm::ModuleSlotTracker &getDiagSlotTracker();
+
/// Clears the accumulated state before processing a new region.
void clearRegionState() {
valueMapping.clear();
@@ -539,6 +548,13 @@ class ModuleImport {
std::unique_ptr<detail::DebugImporter> debugImporter;
/// Loop annotation importer.
std::unique_ptr<detail::LoopAnnotationImporter> loopAnnotationImporter;
+ /// Slot tracker shared by every diagnostic that renders an LLVM entity.
+ /// Metadata::print and Value::print each construct their own tracker when
+ /// not given one, and building one walks the module, so a per-call tracker
+ /// makes a per-instruction diagnostic quadratic in module size. Sharing one
+ /// makes that walk a per-import cost instead. Created lazily, so an import
+ /// that renders nothing never pays for it at all.
+ std::unique_ptr<llvm::ModuleSlotTracker> diagSlotTracker;
/// An option to control if expensive but uncritical diagnostics should be
/// emitted. Avoids generating warnings for unhandled debug intrinsics and
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index c51d03fca1753e..2e4098425cda17 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -41,6 +41,7 @@
#include "llvm/IR/Instructions.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Metadata.h"
+#include "llvm/IR/ModuleSlotTracker.h"
#include "llvm/IR/Operator.h"
#include "llvm/Support/LogicalResult.h"
#include "llvm/Support/ModRef.h"
@@ -52,25 +53,61 @@ using namespace mlir::LLVM::detail;
#include "mlir/Dialect/LLVMIR/LLVMConversionEnumsFromLLVM.inc"
-// Utility to print an LLVM value as a string for passing to emitError().
-// FIXME: Diagnostic should be able to natively handle types that have
-// operator << (raw_ostream&) defined.
-static std::string diag(const llvm::Value &value) {
+/// Upper bound on the characters a diagnostic renders for an LLVM entity.
+/// One rendering is unbounded by nature: a GlobalVariable prints its entire
+/// initializer, and a node with long MDStrings or large inline constants
+/// prints them whole. On a merged library a single unbounded diagnostic
+/// reaches tens of megabytes, so every rendering path below truncates its
+/// output. Diagnostics only need to identify the entity; the full form is
+/// in the input being imported.
+static constexpr size_t kMaxDiagLength = 256;
+
+static std::string truncateDiag(std::string str) {
+ if (str.size() <= kMaxDiagLength)
+ return str;
+ str.resize(kMaxDiagLength);
+ str += "... <truncated>";
+ return str;
+}
+
+// Prints a metadata node through a caller-provided tracker so module-flag
+// conversion shares the importer's tracker instead of building one per call.
+static std::string renderMetadataWithTracker(const llvm::Metadata *node,
+ llvm::ModuleSlotTracker &tracker,
+ const llvm::Module *module) {
std::string str;
llvm::raw_string_ostream os(str);
- os << value;
- return str;
+ node->print(os, tracker, module, /*IsForDebug=*/true);
+ return truncateDiag(std::move(str));
+}
+
+llvm::ModuleSlotTracker &ModuleImport::getDiagSlotTracker() {
+ // Printing through a caller-provided tracker numbers exactly like the
+ // per-call tracker Metadata::print and Value::print would construct, but
+ // the tracker's lazy module pre-pass is paid once per import instead of
+ // once per rendering.
+ if (!diagSlotTracker)
+ diagSlotTracker =
+ std::make_unique<llvm::ModuleSlotTracker>(llvmModule.get());
+ return *diagSlotTracker;
}
-// Utility to print an LLVM metadata node as a string for passing
-// to emitError(). The module argument is needed to print the nodes
-// canonically numbered.
-static std::string diagMD(const llvm::Metadata *node,
- const llvm::Module *module) {
+std::string ModuleImport::renderValue(const llvm::Value &value) {
std::string str;
llvm::raw_string_ostream os(str);
- node->print(os, module, /*IsForDebug=*/true);
- return str;
+ // A GlobalValue renders its full definition. For a merged vtable or string
+ // table that is megabytes, so print globals by name only.
+ if (const auto *global = llvm::dyn_cast<llvm::GlobalValue>(&value)) {
+ global->printAsOperand(os, /*PrintType=*/false, getDiagSlotTracker());
+ return truncateDiag(std::move(str));
+ }
+ value.print(os, getDiagSlotTracker());
+ return truncateDiag(std::move(str));
+}
+
+std::string ModuleImport::renderMetadata(const llvm::Metadata *node) {
+ return renderMetadataWithTracker(node, getDiagSlotTracker(),
+ llvmModule.get());
}
/// Returns the name of the global_ctors global variables.
@@ -284,6 +321,8 @@ getTopologicallySortedBlocks(ArrayRef<llvm::BasicBlock *> basicBlocks) {
return blocks;
}
+ModuleImport::~ModuleImport() = default;
+
ModuleImport::ModuleImport(ModuleOp mlirModule,
std::unique_ptr<llvm::Module> llvmModule,
bool emitExpensiveWarnings,
@@ -367,16 +406,15 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
const auto *memberNode =
dyn_cast<const llvm::MDNode>(node->getOperand(2 * pairNum + 1));
if (!memberNode) {
- emitError(loc) << "operand '" << 2 * pairNum + 1 << "' must be MDNode: "
- << diagMD(node, llvmModule.get());
+ emitError(loc) << "operand '" << 2 * pairNum + 1
+ << "' must be MDNode: " << renderMetadata(node);
return false;
}
int64_t offset = 0;
if (2 * pairNum + 2 >= numOperands) {
// Allow for optional 0 offset in 2-operand nodes.
if (numOperands != 2) {
- emitError(loc) << "missing member offset: "
- << diagMD(node, llvmModule.get());
+ emitError(loc) << "missing member offset: " << renderMetadata(node);
return false;
}
} else {
@@ -384,8 +422,7 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
node->getOperand(2 * pairNum + 2));
if (!offsetCI) {
emitError(loc) << "operand '" << 2 * pairNum + 2
- << "' must be ConstantInt: "
- << diagMD(node, llvmModule.get());
+ << "' must be ConstantInt: " << renderMetadata(node);
return false;
}
offset = offsetCI->getZExtValue();
@@ -441,7 +478,7 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
llvm::mdconst::dyn_extract<llvm::ConstantInt>(node->getOperand(3));
if (!isConstantCI) {
emitError(loc) << "operand '3' must be ConstantInt: "
- << diagMD(node, llvmModule.get());
+ << renderMetadata(node);
return false;
}
isConst = isConstantCI->getValue()[0];
@@ -488,8 +525,8 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
// and we have a cycle in the graph. Cycles are not allowed in TBAA
// graphs.
if (!seen.insert(current).second)
- return emitError(loc) << "has cycle in TBAA graph: "
- << diagMD(current, llvmModule.get());
+ return emitError(loc)
+ << "has cycle in TBAA graph: " << renderMetadata(current);
continue;
}
@@ -533,7 +570,7 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
}
return emitError(loc) << "unsupported TBAA node format: "
- << diagMD(current, llvmModule.get());
+ << renderMetadata(current);
}
return success();
}
@@ -543,7 +580,7 @@ ModuleImport::processAccessGroupMetadata(const llvm::MDNode *node) {
Location loc = mlirModule.getLoc();
if (failed(loopAnnotationImporter->translateAccessGroup(node, loc)))
return emitError(loc) << "unsupported access group node: "
- << diagMD(node, llvmModule.get());
+ << renderMetadata(node);
return success();
}
@@ -597,11 +634,11 @@ ModuleImport::processAliasScopeMetadata(const llvm::MDNode *node) {
// node before.
if (!verifySelfRefOrString(scope) || !domain ||
!verifyDescription(scope, 2))
- return emitError(loc) << "unsupported alias scope node: "
- << diagMD(scope, llvmModule.get());
+ return emitError(loc)
+ << "unsupported alias scope node: " << renderMetadata(scope);
if (!verifySelfRefOrString(domain) || !verifyDescription(domain, 2))
- return emitError(loc) << "unsupported alias domain node: "
- << diagMD(domain, llvmModule.get());
+ return emitError(loc)
+ << "unsupported alias domain node: " << renderMetadata(domain);
if (aliasScopeMapping.contains(scope))
continue;
@@ -690,11 +727,13 @@ static Attribute convertCGProfileModuleFlagValue(ModuleOp mlirModule,
/// something else is found.
static llvm::MDTuple *getTwoElementMDTuple(ModuleOp mlirModule,
const llvm::Module *llvmModule,
- const llvm::MDOperand &md) {
+ const llvm::MDOperand &md,
+ llvm::ModuleSlotTracker &tracker) {
auto *tupleEntry = dyn_cast_or_null<llvm::MDTuple>(md);
if (!tupleEntry || tupleEntry->getNumOperands() != 2)
emitWarning(mlirModule.getLoc())
- << "expected 2-element tuple metadata: " << diagMD(md, llvmModule);
+ << "expected 2-element tuple metadata: "
+ << renderMetadataWithTracker(md, tracker, llvmModule);
return tupleEntry;
}
@@ -703,8 +742,10 @@ static llvm::MDTuple *getTwoElementMDTuple(ModuleOp mlirModule,
/// `matchKey` is different from the tuple's key.
static llvm::ConstantAsMetadata *getConstantMDFromKeyValueTuple(
ModuleOp mlirModule, const llvm::Module *llvmModule,
- const llvm::MDOperand &md, StringRef matchKey, bool optional = false) {
- llvm::MDTuple *tupleEntry = getTwoElementMDTuple(mlirModule, llvmModule, md);
+ const llvm::MDOperand &md, StringRef matchKey,
+ llvm::ModuleSlotTracker &tracker, bool optional = false) {
+ llvm::MDTuple *tupleEntry =
+ getTwoElementMDTuple(mlirModule, llvmModule, md, tracker);
if (!tupleEntry)
return nullptr;
auto *keyMD = dyn_cast<llvm::MDString>(tupleEntry->getOperand(0));
@@ -712,7 +753,8 @@ static llvm::ConstantAsMetadata *getConstantMDFromKeyValueTuple(
if (!optional)
emitWarning(mlirModule.getLoc())
<< "expected '" << matchKey << "' key, but found: "
- << diagMD(tupleEntry->getOperand(0), llvmModule);
+ << renderMetadataWithTracker(tupleEntry->getOperand(0), tracker,
+ llvmModule);
return nullptr;
}
@@ -725,9 +767,10 @@ static llvm::ConstantAsMetadata *getConstantMDFromKeyValueTuple(
static FailureOr<uint64_t>
convertInt64FromKeyValueTuple(ModuleOp mlirModule,
const llvm::Module *llvmModule,
- const llvm::MDOperand &md, StringRef matchKey) {
- llvm::ConstantAsMetadata *valMD =
- getConstantMDFromKeyValueTuple(mlirModule, llvmModule, md, matchKey);
+ const llvm::MDOperand &md, StringRef matchKey,
+ llvm::ModuleSlotTracker &tracker) {
+ llvm::ConstantAsMetadata *valMD = getConstantMDFromKeyValueTuple(
+ mlirModule, llvmModule, md, matchKey, tracker);
if (!valMD)
return failure();
@@ -736,14 +779,16 @@ convertInt64FromKeyValueTuple(ModuleOp mlirModule,
emitWarning(mlirModule.getLoc())
<< "expected integer metadata value for key '" << matchKey
- << "': " << diagMD(md, llvmModule);
+ << "': " << renderMetadataWithTracker(md, tracker, llvmModule);
return failure();
}
static std::optional<ProfileSummaryFormatKind>
convertProfileSummaryFormat(ModuleOp mlirModule, const llvm::Module *llvmModule,
- const llvm::MDOperand &formatMD) {
- auto *tupleEntry = getTwoElementMDTuple(mlirModule, llvmModule, formatMD);
+ const llvm::MDOperand &formatMD,
+ llvm::ModuleSlotTracker &tracker) {
+ auto *tupleEntry =
+ getTwoElementMDTuple(mlirModule, llvmModule, formatMD, tracker);
if (!tupleEntry)
return std::nullopt;
@@ -751,7 +796,8 @@ convertProfileSummaryFormat(ModuleOp mlirModule, const llvm::Module *llvmModule,
if (!keyMD || keyMD->getString() != "ProfileFormat") {
emitWarning(mlirModule.getLoc())
<< "expected 'ProfileFormat' key: "
- << diagMD(tupleEntry->getOperand(0), llvmModule);
+ << renderMetadataWithTracker(tupleEntry->getOperand(0), tracker,
+ llvmModule);
return std::nullopt;
}
@@ -766,7 +812,7 @@ convertProfileSummaryFormat(ModuleOp mlirModule, const llvm::Module *llvmModule,
if (!valMD) {
emitWarning(mlirModule.getLoc())
<< "expected string metadata value for key 'ProfileFormat': "
- << diagMD(valueMD, llvmModule);
+ << renderMetadataWithTracker(valueMD, tracker, llvmModule);
return std::nullopt;
}
std::optional<ProfileSummaryFormatKind> fmtKind =
@@ -775,7 +821,7 @@ convertProfileSummaryFormat(ModuleOp mlirModule, const llvm::Module *llvmModule,
emitWarning(mlirModule.getLoc())
<< "expected 'SampleProfile', 'InstrProf' or 'CSInstrProf' values, "
"but found: "
- << diagMD(valMD, llvmModule);
+ << renderMetadataWithTracker(valMD, tracker, llvmModule);
return std::nullopt;
}
@@ -785,8 +831,10 @@ convertProfileSummaryFormat(ModuleOp mlirModule, const llvm::Module *llvmModule,
static FailureOr<SmallVector<ModuleFlagProfileSummaryDetailedAttr>>
convertProfileSummaryDetailed(ModuleOp mlirModule,
const llvm::Module *llvmModule,
- const llvm::MDOperand &summaryMD) {
- auto *tupleEntry = getTwoElementMDTuple(mlirModule, llvmModule, summaryMD);
+ const llvm::MDOperand &summaryMD,
+ llvm::ModuleSlotTracker &tracker) {
+ auto *tupleEntry =
+ getTwoElementMDTuple(mlirModule, llvmModule, summaryMD, tracker);
if (!tupleEntry)
return failure();
@@ -794,7 +842,8 @@ convertProfileSummaryDetailed(ModuleOp mlirModule,
if (!keyMD || keyMD->getString() != "DetailedSummary") {
emitWarning(mlirModule.getLoc())
<< "expected 'DetailedSummary' key: "
- << diagMD(tupleEntry->getOperand(0), llvmModule);
+ << renderMetadataWithTracker(tupleEntry->getOperand(0), tracker,
+ llvmModule);
return failure();
}
@@ -802,7 +851,8 @@ convertProfileSummaryDetailed(ModuleOp mlirModule,
if (!entriesMD) {
emitWarning(mlirModule.getLoc())
<< "expected tuple value for 'DetailedSummary' key: "
- << diagMD(tupleEntry->getOperand(1), llvmModule);
+ << renderMetadataWithTracker(tupleEntry->getOperand(1), tracker,
+ llvmModule);
return failure();
}
@@ -812,7 +862,7 @@ convertProfileSummaryDetailed(ModuleOp mlirModule,
if (!entryMD || entryMD->getNumOperands() != 3) {
emitWarning(mlirModule.getLoc())
<< "'DetailedSummary' entry expects 3 operands: "
- << diagMD(entry, llvmModule);
+ << renderMetadataWithTracker(entry, tracker, llvmModule);
return failure();
}
@@ -822,7 +872,7 @@ convertProfileSummaryDetailed(ModuleOp mlirModule,
if (!op0 || !op1 || !op2) {
emitWarning(mlirModule.getLoc())
<< "expected only integer entries in 'DetailedSummary': "
- << diagMD(entry, llvmModule);
+ << renderMetadataWithTracker(entry, tracker, llvmModule);
return failure();
}
@@ -836,15 +886,14 @@ convertProfileSummaryDetailed(ModuleOp mlirModule,
return detailedSummary;
}
-static Attribute
-convertProfileSummaryModuleFlagValue(ModuleOp mlirModule,
- const llvm::Module *llvmModule,
- llvm::MDTuple *mdTuple) {
+static Attribute convertProfileSummaryModuleFlagValue(
+ ModuleOp mlirModule, const llvm::Module *llvmModule, llvm::MDTuple *mdTuple,
+ llvm::ModuleSlotTracker &tracker) {
unsigned profileNumEntries = mdTuple->getNumOperands();
if (profileNumEntries < 8) {
emitWarning(mlirModule.getLoc())
<< "expected at 8 entries in 'ProfileSummary': "
- << diagMD(mdTuple, llvmModule);
+ << renderMetadataWithTracker(mdTuple, tracker, llvmModule);
return nullptr;
}
@@ -857,7 +906,8 @@ convertProfileSummaryModuleFlagValue(ModuleOp mlirModule,
if (summayIdx + 1 >= profileNumEntries) {
emitWarning(mlirModule.getLoc())
<< "the last summary entry is '" << matchKey
- << "', expected 'DetailedSummary': " << diagMD(md, llvmModule);
+ << "', expected 'DetailedSummary': "
+ << renderMetadataWithTracker(md, tracker, llvmModule);
return failure();
}
@@ -868,12 +918,13 @@ convertProfileSummaryModuleFlagValue(ModuleOp mlirModule,
[&](const llvm::MDOperand &md,
StringRef matchKey) -> FailureOr<std::optional<uint64_t>> {
if (!getConstantMDFromKeyValueTuple(mlirModule, llvmModule, md, matchKey,
+ tracker,
/*optional=*/true))
return FailureOr<std::optional<uint64_t>>(std::nullopt);
if (checkOptionalPosition(md, matchKey).failed())
return failure();
- FailureOr<uint64_t> val =
- convertInt64FromKeyValueTuple(mlirModule, llvmModule, md, matchKey);
+ FailureOr<uint64_t> val = convertInt64FromKeyValueTuple(
+ mlirModule, llvmModule, md, matchKey, tracker);
if (failed(val))
return failure();
return val;
@@ -882,7 +933,8 @@ convertProfileSummaryModuleFlagValue(ModuleOp mlirModule,
auto getOptDoubleValue = [&](const llvm::MDOperand &md,
StringRef matchKey) -> FailureOr<FloatAttr> {
auto *valMD = getConstantMDFromKeyValueTuple(mlirModule, llvmModule, md,
- matchKey, /*optional=*/true);
+ matchKey, tracker,
+ /*optional=*/true);
if (!valMD)
return FloatAttr{};
if (auto *cstFP = dyn_cast<llvm::ConstantFP>(valMD->getValue())) {
@@ -893,46 +945,50 @@ convertProfileSummaryModuleFlagValue(ModuleOp mlirModule,
}
emitWarning(mlirModule.getLoc())
<< "expected double metadata value for key '" << matchKey
- << "': " << diagMD(md, llvmModule);
+ << "': " << renderMetadataWithTr...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/229956
More information about the Mlir-commits
mailing list