[Mlir-commits] [mlir] [MLIR][Remark] Add remark linking and query-based related remarks (PR #180953)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Feb 11 06:47:58 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-core
Author: Guray Ozen (grypp)
<details>
<summary>Changes</summary>
Add RemarkId and relatedTo support to the MLIR Remarks infrastructure.
*Example*
```
remark1 = remark::analyzed(...)
remark::passed(loc, ... .relatedTo(remark1))
```
---
Patch is 31.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/180953.diff
5 Files Affected:
- (modified) mlir/include/mlir/IR/Remarks.h (+195-20)
- (modified) mlir/lib/IR/Remarks.cpp (+76-4)
- (modified) mlir/test/Pass/remarks.mlir (+25-15)
- (modified) mlir/test/lib/Pass/TestRemarksPass.cpp (+16)
- (modified) mlir/unittests/IR/RemarkTest.cpp (+145-16)
``````````diff
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index eb0518e203d36..c77d943dad91c 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -23,8 +23,42 @@
#include "mlir/IR/MLIRContext.h"
#include "mlir/IR/Value.h"
+#include <atomic>
+
namespace mlir::remark {
+//===----------------------------------------------------------------------===//
+// RemarkId - Unique identifier for linking related remarks
+//===----------------------------------------------------------------------===//
+
+/// A unique identifier for a remark, used to link related remarks together.
+/// An invalid/empty ID has value 0.
+class RemarkId {
+public:
+ RemarkId() : id(0) {}
+ explicit RemarkId(uint64_t id) : id(id) {}
+
+ /// Check if this is a valid (non-zero) ID.
+ explicit operator bool() const { return id != 0; }
+
+ /// Get the raw ID value.
+ uint64_t getValue() const { return id; }
+
+ bool operator==(const RemarkId &other) const { return id == other.id; }
+ bool operator!=(const RemarkId &other) const { return id != other.id; }
+
+ /// Print the ID.
+ void print(llvm::raw_ostream &os) const { os << "remark-" << id; }
+
+private:
+ uint64_t id;
+};
+
+inline llvm::raw_ostream &operator<<(llvm::raw_ostream &os, RemarkId id) {
+ id.print(os);
+ return os;
+}
+
/// Define an the set of categories to accept. By default none are, the provided
/// regex matches against the category names for each kind of remark.
struct RemarkCategories {
@@ -53,6 +87,10 @@ enum class RemarkKind {
using namespace llvm;
+namespace detail {
+class InFlightRemark; // forward declaration
+} // namespace detail
+
/// Options to create a Remark
struct RemarkOpts {
StringRef remarkName; // Identifiable name
@@ -60,21 +98,72 @@ struct RemarkOpts {
StringRef subCategoryName; // Subcategory name
StringRef functionName; // Function name if available
+ /// Link to a related remark by explicit ID.
+ RemarkId relatedId;
+
+ /// Link to related remarks by query (resolved at emit time).
+ /// Empty means "don't search".
+ StringRef relatedCategory;
+ std::optional<RemarkKind> relatedKind;
+
// Construct RemarkOpts from a remark name.
- static constexpr RemarkOpts name(StringRef n) {
- return RemarkOpts{n, {}, {}, {}};
+ static RemarkOpts name(StringRef n) {
+ RemarkOpts o;
+ o.remarkName = n;
+ return o;
}
+
/// Return a copy with the category set.
- constexpr RemarkOpts category(StringRef v) const {
- return {remarkName, v, subCategoryName, functionName};
+ RemarkOpts category(StringRef v) const {
+ auto copy = *this;
+ copy.categoryName = v;
+ return copy;
}
/// Return a copy with the subcategory set.
- constexpr RemarkOpts subCategory(StringRef v) const {
- return {remarkName, categoryName, v, functionName};
+ RemarkOpts subCategory(StringRef v) const {
+ auto copy = *this;
+ copy.subCategoryName = v;
+ return copy;
}
/// Return a copy with the function name set.
- constexpr RemarkOpts function(StringRef v) const {
- return {remarkName, categoryName, subCategoryName, v};
+ RemarkOpts function(StringRef v) const {
+ auto copy = *this;
+ copy.functionName = v;
+ return copy;
+ }
+
+ /// Link this remark to a previously emitted remark by explicit ID.
+ RemarkOpts relatedTo(RemarkId id) const {
+ auto copy = *this;
+ copy.relatedId = id;
+ return copy;
+ }
+
+ /// Link this remark to a related remark that is still in flight.
+ /// Extracts the ID from the InFlightRemark.
+ inline RemarkOpts relatedTo(const detail::InFlightRemark &r) const;
+
+ /// Link this remark to previously emitted remarks by query.
+ /// Resolved at emit time via the engine's policy.
+ RemarkOpts relatedTo(StringRef cat, RemarkKind kind) const {
+ auto copy = *this;
+ copy.relatedCategory = cat;
+ copy.relatedKind = kind;
+ return copy;
+ }
+
+ /// Link this remark to previously emitted remarks by full RemarkOpts query.
+ RemarkOpts relatedTo(RemarkOpts searchOpts,
+ std::optional<RemarkKind> kind) const {
+ auto copy = *this;
+ copy.relatedCategory = searchOpts.categoryName;
+ copy.relatedKind = kind;
+ return copy;
+ }
+
+ /// Check if this opts has any relatedTo configuration.
+ bool hasRelatedTo() const {
+ return static_cast<bool>(relatedId) || !relatedCategory.empty();
}
};
@@ -97,6 +186,9 @@ class Remark {
(llvm::Twine(categoryName) + ":" + subCategoryName)
.toVector(fullCategoryName);
}
+ // Explicit ID linking from opts.
+ if (opts.relatedId)
+ addRelatedRemark(opts.relatedId);
}
// Remark argument that is a key-value pair that can be printed as machine
@@ -178,6 +270,38 @@ class Remark {
llvm::remarks::Type getRemarkType() const;
+ //===--------------------------------------------------------------------===//
+ // Remark Linking Support
+ //===--------------------------------------------------------------------===//
+
+ /// Get this remark's unique ID (0 if not assigned).
+ RemarkId getId() const { return id; }
+
+ /// Set this remark's unique ID. Also adds it as an Arg for serialization.
+ void setId(RemarkId newId) {
+ id = newId;
+ if (id)
+ args.emplace_back("RemarkId", llvm::utostr(id.getValue()));
+ }
+
+ /// Get the list of related remark IDs.
+ ArrayRef<RemarkId> getRelatedRemarkIds() const { return relatedRemarks; }
+
+ /// Add a reference to a related remark. Also adds it as an Arg for
+ /// serialization.
+ void addRelatedRemark(RemarkId relatedId) {
+ if (relatedId) {
+ relatedRemarks.push_back(relatedId);
+ args.emplace_back("RelatedTo", llvm::utostr(relatedId.getValue()));
+ }
+ }
+
+ /// Check if this remark has any related remarks.
+ bool hasRelatedRemarks() const { return !relatedRemarks.empty(); }
+
+ /// Get the remark kind.
+ RemarkKind getRemarkKind() const { return remarkKind; }
+
StringRef getRemarkTypeString() const;
protected:
@@ -207,6 +331,12 @@ class Remark {
/// Args collected via the streaming interface.
SmallVector<Arg, 4> args;
+ /// Unique ID for this remark (assigned by RemarkEngine).
+ RemarkId id;
+
+ /// IDs of related remarks (e.g., parent analysis that enabled this opt).
+ SmallVector<RemarkId> relatedRemarks;
+
private:
/// Convert the MLIR diagnostic severity to LLVM diagnostic severity.
static llvm::DiagnosticSeverity
@@ -289,6 +419,9 @@ struct LazyTextBuild {
std::function<std::string()> thunk;
};
+/// A wrapper for linking remarks by query - searches the engine's registry
+/// at stream time and links to all matching remarks.
+
/// InFlightRemark is a RAII class that holds a reference to a Remark
/// instance and allows to build the remark using the << operator. The remark
/// is emitted when the InFlightRemark instance is destroyed, which happens
@@ -321,6 +454,9 @@ class InFlightRemark {
explicit operator bool() const { return remark != nullptr; }
+ /// Get this remark's unique ID (for linking from other remarks).
+ RemarkId getId() const { return remark ? remark->getId() : RemarkId(); }
+
~InFlightRemark();
InFlightRemark(const InFlightRemark &) = delete;
@@ -372,6 +508,15 @@ class RemarkEmittingPolicyBase {
virtual void reportRemark(const Remark &remark) = 0;
virtual void finalize() = 0;
+
+ /// Find previously reported remarks matching the given criteria.
+ /// Default returns empty -- only policies that store remarks (like
+ /// PolicyFinal) override this to enable query-based linking.
+ virtual SmallVector<RemarkId>
+ findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind = std::nullopt) const {
+ return {};
+ }
};
//===----------------------------------------------------------------------===//
@@ -395,6 +540,8 @@ class RemarkEngine {
std::unique_ptr<RemarkEmittingPolicyBase> remarkEmittingPolicy;
/// When is enabled, engine also prints remarks as mlir::emitRemarks.
bool printAsEmitRemarks = false;
+ /// Atomic counter for generating unique remark IDs.
+ std::atomic<uint64_t> nextRemarkId{1};
/// Return true if missed optimization remarks are enabled, override
/// to provide different implementation.
@@ -423,8 +570,8 @@ class RemarkEngine {
/// Emit a remark using the given maker function, which should return
/// a Remark instance. The remark will be emitted using the main
/// remark streamer.
- template <typename RemarkT, typename... Args>
- InFlightRemark makeRemark(Args &&...args);
+ template <typename RemarkT>
+ InFlightRemark makeRemark(Location loc, RemarkOpts opts);
template <typename RemarkT>
InFlightRemark emitIfEnabled(Location loc, RemarkOpts opts,
@@ -457,6 +604,22 @@ class RemarkEngine {
return remarkEmittingPolicy.get();
}
+ /// Generate a unique ID for a new remark.
+ RemarkId generateRemarkId() {
+ return RemarkId(nextRemarkId.fetch_add(1, std::memory_order_relaxed));
+ }
+
+ //===--------------------------------------------------------------------===//
+ // Remark Linking - query previously emitted remarks
+ //===--------------------------------------------------------------------===//
+
+ /// Find all remarks matching the given criteria. Delegates to the
+ /// emitting policy. Only works with policies that store remarks
+ /// (e.g. RemarkEmittingPolicyFinal); returns empty for PolicyAll.
+ SmallVector<RemarkId>
+ findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind = std::nullopt) const;
+
/// Report a remark.
void report(const Remark &&remark);
@@ -491,6 +654,12 @@ inline InFlightRemark withEngine(Fn fn, Location loc, Args &&...args) {
} // namespace mlir::remark::detail
+// Deferred definition: needs InFlightRemark to be complete.
+inline mlir::remark::RemarkOpts
+mlir::remark::RemarkOpts::relatedTo(const detail::InFlightRemark &r) const {
+ return relatedTo(r.getId());
+}
+
namespace mlir::remark {
//===----------------------------------------------------------------------===//
@@ -509,7 +678,8 @@ class RemarkEmittingPolicyAll : public detail::RemarkEmittingPolicyBase {
void finalize() override {}
};
-/// Policy that emits final remarks.
+/// Policy that emits final remarks. Stores all remarks until finalize(),
+/// which enables query-based linking via findRemarks().
class RemarkEmittingPolicyFinal : public detail::RemarkEmittingPolicyBase {
private:
/// user can intercept them for custom processing via a registered callback,
@@ -524,13 +694,15 @@ class RemarkEmittingPolicyFinal : public detail::RemarkEmittingPolicyBase {
postponedRemarks.insert(remark);
}
- void finalize() override {
- assert(reportImpl && "reportImpl is not set");
- for (auto &remark : postponedRemarks) {
- if (reportImpl)
- reportImpl(remark);
- }
- }
+ /// Emits all stored remarks. Related remarks are printed as nested notes
+ /// under the remark that references them.
+ void finalize() override;
+
+ /// Search stored remarks by RemarkOpts fields + optional kind.
+ /// Empty opts fields are ignored (don't filter on that field).
+ SmallVector<RemarkId>
+ findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind = std::nullopt) const override;
};
/// Create a Reason with llvm::formatv formatting.
@@ -559,6 +731,7 @@ inline detail::LazyTextBuild metric(StringRef key, V &&v) {
return detail::Remark::Arg(key, std::move(vv)).val;
}};
}
+
//===----------------------------------------------------------------------===//
// Emitters
//===----------------------------------------------------------------------===//
@@ -640,7 +813,8 @@ struct DenseMapInfo<mlir::remark::detail::Remark> {
return llvm::hash_combine(
remark.getLocation().getAsOpaquePointer(),
llvm::hash_value(remark.getRemarkName()),
- llvm::hash_value(remark.getCombinedCategoryName()));
+ llvm::hash_value(remark.getCombinedCategoryName()),
+ static_cast<unsigned>(remark.getRemarkKind()));
}
static bool isEqual(const mlir::remark::detail::Remark &lhs,
@@ -656,7 +830,8 @@ struct DenseMapInfo<mlir::remark::detail::Remark> {
// For regular remarks, compare key identifying fields
return lhs.getLocation() == rhs.getLocation() &&
lhs.getRemarkName() == rhs.getRemarkName() &&
- lhs.getCombinedCategoryName() == rhs.getCombinedCategoryName();
+ lhs.getCombinedCategoryName() == rhs.getCombinedCategoryName() &&
+ lhs.getRemarkKind() == rhs.getRemarkKind();
}
};
} // namespace llvm
diff --git a/mlir/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp
index 197cf421f9ba4..7ea3a4cedb477 100644
--- a/mlir/lib/IR/Remarks.cpp
+++ b/mlir/lib/IR/Remarks.cpp
@@ -149,6 +149,7 @@ llvm::remarks::Remark Remark::generateRemark() const {
r.PassName = getCombinedCategoryName();
r.FunctionName = getFunction();
r.Loc = locLambda();
+ // Add all args (includes RemarkId and RelatedTo if they were added).
for (const Remark::Arg &arg : getArgs()) {
r.Args.emplace_back();
r.Args.back().Key = arg.key;
@@ -171,12 +172,33 @@ InFlightRemark::~InFlightRemark() {
// Remark Engine
//===----------------------------------------------------------------------===//
-template <typename RemarkT, typename... Args>
-InFlightRemark RemarkEngine::makeRemark(Args &&...args) {
+template <typename RemarkT>
+InFlightRemark RemarkEngine::makeRemark(Location loc, RemarkOpts opts) {
static_assert(std::is_base_of_v<Remark, RemarkT>,
"RemarkT must derive from Remark");
- return InFlightRemark(*this,
- std::make_unique<RemarkT>(std::forward<Args>(args)...));
+ auto remarkPtr = std::make_unique<RemarkT>(loc, opts);
+ remarkPtr->setId(generateRemarkId());
+
+ // Query-based linking (needs engine access to search the policy).
+ // Explicit ID linking is handled by the Remark constructor.
+ if (!opts.relatedCategory.empty()) {
+ RemarkOpts searchOpts;
+ searchOpts.categoryName = opts.relatedCategory;
+ for (auto id : findRemarks(searchOpts, opts.relatedKind))
+ remarkPtr->addRelatedRemark(id);
+ }
+
+ return InFlightRemark(*this, std::move(remarkPtr));
+}
+
+/// Engine delegates to the emitting policy. PolicyFinal searches its stored
+/// remarks; PolicyAll (and others) return empty.
+SmallVector<RemarkId>
+RemarkEngine::findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind) const {
+ if (remarkEmittingPolicy)
+ return remarkEmittingPolicy->findRemarks(opts, kind);
+ return {};
}
template <typename RemarkT>
@@ -345,4 +367,54 @@ llvm::LogicalResult mlir::remark::enableOptimizationRemarks(
namespace mlir::remark {
RemarkEmittingPolicyAll::RemarkEmittingPolicyAll() = default;
RemarkEmittingPolicyFinal::RemarkEmittingPolicyFinal() = default;
+
+void RemarkEmittingPolicyFinal::finalize() {
+ assert(reportImpl && "reportImpl is not set");
+
+ // Build ID -> Remark* lookup for resolving related remark references.
+ llvm::DenseMap<uint64_t, const detail::Remark *> idMap;
+ llvm::DenseSet<uint64_t> childIds; // IDs referenced as children
+
+ for (const auto &remark : postponedRemarks) {
+ if (remark.getId())
+ idMap[remark.getId().getValue()] = &remark;
+ for (auto relId : remark.getRelatedRemarkIds())
+ childIds.insert(relId.getValue());
+ }
+
+ // Emit remarks with related remarks grouped after their parents.
+ // Parent remarks are emitted first, followed by their related (child)
+ // remarks. Child-only remarks are skipped at the top level to avoid
+ // duplication.
+ for (const auto &remark : postponedRemarks) {
+ if (remark.getId() && childIds.count(remark.getId().getValue()))
+ continue; // will be printed grouped under its parent
+
+ reportImpl(remark);
+
+ // Emit related remarks immediately after the parent.
+ for (auto relId : remark.getRelatedRemarkIds()) {
+ if (const auto *related = idMap.lookup(relId.getValue()))
+ reportImpl(*related);
+ }
+ }
+}
+
+SmallVector<RemarkId>
+RemarkEmittingPolicyFinal::findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind) const {
+ SmallVector<RemarkId> results;
+ for (const auto &remark : postponedRemarks) {
+ // Empty string means "don't filter on this field".
+ if (!opts.categoryName.empty() &&
+ remark.getCategoryName() != opts.categoryName)
+ continue;
+ if (!opts.remarkName.empty() && remark.getRemarkName() != opts.remarkName)
+ continue;
+ if (kind && remark.getRemarkKind() != *kind)
+ continue;
+ results.push_back(remark.getId());
+ }
+ return results;
+}
} // namespace mlir::remark
diff --git a/mlir/test/Pass/remarks.mlir b/mlir/test/Pass/remarks.mlir
index 8aa04e3c98d80..d80469a5b3fbe 100644
--- a/mlir/test/Pass/remarks.mlir
+++ b/mlir/test/Pass/remarks.mlir
@@ -4,25 +4,35 @@
// RUN: mlir-opt %s --test-remark --remarks-filter-analyse="category-2-analysis" 2>&1 | FileCheck %s -check-prefix=CHECK-ANALYSIS
// RUN: mlir-opt %s --test-remark --remarks-filter="category.*" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL
// RUN: mlir-opt %s --test-remark --remarks-filter="category-1.*" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL1
+// RUN: mlir-opt %s --test-remark --remark-policy=final --remarks-filter-passed="category-link" --remarks-filter-analyse="category-link" 2>&1 | FileCheck %s -check-prefix=CHECK-LINK
module @foo {
"test.op"() : () -> ()
-
}
+// Each remark now includes a RemarkId=N arg and remark-N prefix, so use
+// {{.*}} to absorb them.
+// CHECK-PASSED: remark: [Passed]{{.*}}test-remark | Category:category-1-passed |{{.*}}Remark="This is a test passed remark"
+// CHECK-MISSED: remark: [Missed]{{.*}}test-remark | Category:a-category-1-missed |{{.*}}Remark="This is a test missed remark"
+// CHECK-FAILED: remark: [Failure]{{.*}}test-remark | Category:category-2-failed |{{.*}}Remark="This is a test failed remark"
+// CHECK-ANALYSIS: remark: [Analysis]{{.*}}test-remark | Category:category-2-analysis |{{.*}}Remark="This is a test analysis remark"
-// CHECK-PASSED: remarks.mlir:8:3: remark: [Passed] test-remark | Category:category-1-passed | Reason="because we are testing the remark pipeline", Remark="This is a test passed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-MISSED:remarks.mlir:8:3: remark: [Missed] test-remark | Category:a-category-1-missed | Reason="because we are testing the remark pipeline", Remark="This is a test missed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-FAILED: remarks.mlir:8:3: remark: [Failure] test-remark | Category:category-2-failed | Reason="because we are testing the remark pipeline", Remark="This is a test failed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-ANALYSIS: remarks.mlir:8:3: remark: [Analysis] test-remark | Category:category-2-analysis | Remark="This is a test analysis remark"
-
-
-// CHECK-ALL: remarks.mlir:8:3: remark: [Passed] test-remark | Category:category-1-passed | Reason="because we are testing the remark pipeline", Remark="This is a test passed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-ALL: remarks.mlir:8:3: remark: [Failure] test-remark | Category:category-2-failed | Reason="because we are testing the remark pipeline", Remark="This is a test failed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-ALL: remarks.mlir:8:3: remark: [Analysis] test-remark | Category:category-2-analysis | Remark="This is a test analysis remark"
-
-// CHECK-ALL1: remarks.mlir:8:3: remark: [Passed] test-remark | Category:category-1-passed | Reason="because we are testing the remark pipeline", Remark="This is a test passed remark", Suggestion="try using the remark pipeline feature"
-// CHECK-ALL1-NOT: remarks.mlir:8:3: remark: [Missed]
-// CHECK-ALL1-NOT: remarks.mlir:8:3: remark: [Failure]
-// CHECK-ALL1-NOT: remarks.mlir:8:3: remark: [Analysis]
+// CHECK-ALL: remark: [Passed]{{.*}}test-remark | Category:category-1-passed |
+// CHECK-ALL: remark: [Passed]{{.*}}test-remark | Category:category-1-passed |
+// CHECK-ALL: remark: [Failure]{{.*}}test-remark | Category:category-2-failed |
+// CHECK-ALL: remark: [Analysis]{{.*}}test-remark | Category:category-2-analysis |
+// CHECK-...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/180953
More information about the Mlir-commits
mailing list