[Mlir-commits] [mlir] [mlir] Add searchable remark history and query-based links (PR #237762)
Guray Ozen
llvmlistbot at llvm.org
Tue Nov 3 01:50:03 PST 2026
https://github.com/grypp updated https://github.com/llvm/llvm-project/pull/237762
>From 0f00bce6a7390bacf23585a31b7b288524eb4d7e Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Sun, 1 Nov 2026 19:43:51 +0100
Subject: [PATCH 1/2] [mlir] Add searchable remark history and query-based
links
---
mlir/docs/Remarks.md | 85 +++++++++
mlir/include/mlir/IR/Remarks.h | 113 ++++++++++--
mlir/lib/IR/Remarks.cpp | 83 +++++++++
mlir/unittests/IR/RemarkTest.cpp | 298 +++++++++++++++++++++++++++++++
4 files changed, 562 insertions(+), 17 deletions(-)
diff --git a/mlir/docs/Remarks.md b/mlir/docs/Remarks.md
index e747f7bc8f5958..663eb1161e9ad8 100644
--- a/mlir/docs/Remarks.md
+++ b/mlir/docs/Remarks.md
@@ -251,6 +251,91 @@ if (engine->isRemarkEnabled(remark::RemarkKind::RemarkPassed, "Vectorizer"))
***
+## Searching and Linking Remarks
+
+Remarks can refer to other remarks by ID using `RemarkOpts::relatedTo(id)`.
+When the producer does not have the ID, it can search previously reported
+remarks and link to the matches.
+
+First, enable history on an already configured engine:
+
+```c++
+auto *engine = context.getRemarkEngine();
+engine->enableHistory();
+```
+
+History is disabled by default. Enabling it retains the IDs and identifying
+metadata of subsequent reports, without retaining their messages or arguments.
+It does not recover earlier reports or remarks excluded by the engine's
+category and kind filters. An in-flight remark becomes searchable when it is
+reported at the end of its lifetime.
+
+Use `RemarkQuery` to match a name, base category, subcategory, function, kind,
+or location. Specified fields are combined with AND and match exactly, without
+regular expressions. Unspecified fields match anything; an explicitly empty
+string matches empty metadata. Location matching compares the complete MLIR
+location, including wrappers, and does not identify a unique operation.
+
+```c++
+auto analysisOpts = remark::RemarkOpts::name("VectorSizeInference")
+ .category("Vectorizer")
+ .function("kernel");
+remark::analysis(loc, analysisOpts)
+ << remark::reason("Unable to infer vector sizes");
+
+auto query = remark::RemarkQuery()
+ .name("VectorSizeInference")
+ .category("Vectorizer")
+ .function("kernel")
+ .kind(remark::RemarkKind::RemarkAnalysis)
+ .at(loc);
+
+// Obtain the IDs explicitly, in report order.
+SmallVector<remark::RemarkId> matches = engine->findRemarks(query);
+
+// Or link all matching reports without handling their IDs.
+remark::missed(loc, remark::RemarkOpts::name("Vectorization")
+ .category("Vectorizer")
+ .function("kernel"))
+ << remark::reason("Cannot vectorize without vector sizes")
+ << remark::relatedTo(query);
+```
+
+The helper resolves the query when inserted with `<<`. Later reports do not
+change the links. No matches, including when history is disabled, adds no
+links. Links use the existing repeated `RelatedTo` arguments in serialized
+output. Consumers must preserve repeated keys. A query match indicates a
+relationship, not causality; the producer is responsible for choosing relevant
+criteria. There is no automatic selection of the latest match or inference of
+whether earlier evidence is still valid for changed IR.
+
+Queries own their strings, so they can be retained and reused. A query with a
+location must not outlive that location's `MLIRContext`. History access is
+synchronized, and streamer callbacks may query the history, including the
+report being delivered. This does not synchronize the emitting policy or
+streamer. Report order follows insertion into history and can differ from ID
+creation order, including when remarks are emitted concurrently.
+
+History is independent of the output policy: both `All` and `Final` record
+every report, and finalizing a policy does not clear history. Use `All` when
+the output must contain every referenced event. `Final` may deduplicate a
+referenced report or emit it in an earlier batch, so finding its ID in history
+does not guarantee that the current output batch contains it.
+
+History grows with the number of reports until explicitly cleared, disabled,
+or the engine is destroyed. Clear it between compilations when reusing an
+engine:
+
+```c++
+engine->clearHistory(); // Keep recording subsequent reports.
+engine->enableHistory(false); // Discard history and stop recording.
+```
+
+Neither operation resets the ID counter or changes existing links. Repeated
+calls to `enableHistory()` preserve already recorded history.
+
+***
+
## Enabling Remarks
### Option 1: LLVM Remark Streamer (YAML or Bitstream)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 0ecc63717b5aee..6acbc722944ff4 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -24,6 +24,7 @@
#include "mlir/IR/Value.h"
#include <atomic>
+#include <mutex>
namespace mlir::remark {
@@ -89,6 +90,7 @@ using namespace llvm;
namespace detail {
class InFlightRemark; // forward declaration
+class RemarkEngine;
} // namespace detail
/// Options to create a Remark
@@ -139,6 +141,52 @@ struct RemarkOpts {
inline RemarkOpts relatedTo(const detail::InFlightRemark &r) const;
};
+/// Exact-match criteria for searching previously reported remarks. Unspecified
+/// fields match any value; a specified empty string matches an empty field.
+/// Strings are owned by the query. Locations belong to their MLIRContext and
+/// must not outlive it. A location match does not imply operation identity.
+class RemarkQuery {
+public:
+ RemarkQuery name(StringRef value) const {
+ auto copy = *this;
+ copy.remarkName = value.str();
+ return copy;
+ }
+ /// Match the base category, without the subcategory.
+ RemarkQuery category(StringRef value) const {
+ auto copy = *this;
+ copy.categoryName = value.str();
+ return copy;
+ }
+ RemarkQuery subCategory(StringRef value) const {
+ auto copy = *this;
+ copy.subCategoryName = value.str();
+ return copy;
+ }
+ RemarkQuery function(StringRef value) const {
+ auto copy = *this;
+ copy.functionName = value.str();
+ return copy;
+ }
+ RemarkQuery kind(RemarkKind value) const {
+ auto copy = *this;
+ copy.remarkKind = value;
+ return copy;
+ }
+ /// Match the complete location, including any wrapping locations.
+ RemarkQuery at(Location value) const {
+ auto copy = *this;
+ copy.location = value;
+ return copy;
+ }
+
+private:
+ friend class detail::RemarkEngine;
+ std::optional<std::string> remarkName, categoryName, subCategoryName,
+ functionName;
+ std::optional<RemarkKind> remarkKind;
+ std::optional<Location> location;
+};
} // namespace mlir::remark
namespace mlir::remark::detail {
@@ -146,6 +194,7 @@ namespace mlir::remark::detail {
// Remark Base Class
//===----------------------------------------------------------------------===//
class Remark {
+ friend class RemarkEngine;
public:
Remark(RemarkKind remarkKind, DiagnosticSeverity severity, Location loc,
@@ -391,8 +440,10 @@ 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.
+/// Resolves a query when inserted into an in-flight remark.
+struct RelatedRemarkQuery {
+ RemarkQuery query;
+};
/// InFlightRemark is a RAII class that holds a reference to a Remark
/// instance and allows to build the remark using the << operator. The remark
@@ -415,9 +466,13 @@ class InFlightRemark {
return *this;
}
- // Generic path, but *not* for Lazy
- template <typename T, typename = std::enable_if_t<
- !std::is_same_v<std::decay_t<T>, LazyTextBuild>>>
+ InFlightRemark &operator<<(const RelatedRemarkQuery &related);
+
+ // Generic path, excluding wrappers handled by dedicated overloads.
+ template <typename T,
+ typename = std::enable_if_t<
+ !std::is_same_v<std::decay_t<T>, LazyTextBuild> &&
+ !std::is_same_v<std::decay_t<T>, RelatedRemarkQuery>>>
InFlightRemark &operator<<(T &&arg) {
if (remark)
*remark << std::forward<T>(arg);
@@ -480,15 +535,6 @@ 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 {};
- }
};
//===----------------------------------------------------------------------===//
@@ -515,6 +561,13 @@ class RemarkEngine {
/// Atomic counter for generating unique remark IDs.
std::atomic<uint64_t> nextRemarkId{1};
+ /// Search history is independent of the emitting policy. Only metadata is
+ /// retained, and only after explicitly enabling history.
+ struct RemarkHistory;
+ std::unique_ptr<RemarkHistory> history;
+ std::atomic<bool> historyEnabled{false};
+ mutable std::mutex historyMutex;
+
/// Emit a remark using the given maker function, which should return
/// a Remark instance. The remark will be emitted using the main
/// remark streamer.
@@ -600,9 +653,28 @@ class RemarkEngine {
// 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.
+ /// Enable recording metadata for subsequent reports. Repeated enabling
+ /// preserves history. Disabling discards history. No past events are
+ /// recovered. History is retained until cleared, disabled, or the engine is
+ /// destroyed; callers reusing an engine across compilations should clear it
+ /// between runs.
+ void enableHistory(bool enable = true);
+
+ /// Discard recorded metadata without changing whether history is enabled.
+ /// Existing remark IDs and links are unaffected.
+ void clearHistory();
+
+ /// Find all recorded remarks matching every specified criterion, in report
+ /// order. Queries see a snapshot of history, including reports not yet
+ /// emitted by a deferred policy. In-flight and filtered-out remarks are not
+ /// recorded. Returns empty when history is disabled. Policy finalization does
+ /// not clear history. History access is synchronized; this does not make
+ /// emitting policies or streamers thread-safe.
+ SmallVector<RemarkId> findRemarks(const RemarkQuery &query) const;
+
+ /// Search using nonempty option fields as exact-match criteria. The related
+ /// ID in opts is ignored. Use RemarkQuery to match empty fields or a
+ /// location.
SmallVector<RemarkId>
findRemarks(const RemarkOpts &opts,
std::optional<RemarkKind> kind = std::nullopt) const;
@@ -648,6 +720,13 @@ mlir::remark::RemarkOpts::relatedTo(const detail::InFlightRemark &r) const {
}
namespace mlir::remark {
+/// Link to all remarks matching the query when this helper is inserted into an
+/// in-flight remark. History must have been enabled before reporting the target
+/// remarks. No matches (including disabled history) adds no links. Matching
+/// indicates a relationship, not causality or operation identity.
+inline detail::RelatedRemarkQuery relatedTo(RemarkQuery query) {
+ return {std::move(query)};
+}
//===----------------------------------------------------------------------===//
// Remark Emitting Policies
diff --git a/mlir/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp
index 14d095824f6712..e3f90ebf617b89 100644
--- a/mlir/lib/IR/Remarks.cpp
+++ b/mlir/lib/IR/Remarks.cpp
@@ -15,6 +15,8 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include <vector>
+
using namespace mlir::remark::detail;
using namespace mlir::remark;
//------------------------------------------------------------------------------
@@ -164,6 +166,13 @@ llvm::remarks::Remark Remark::generateRemark() const {
// InFlightRemark
//===----------------------------------------------------------------------===//
+InFlightRemark &InFlightRemark::operator<<(const RelatedRemarkQuery &related) {
+ if (remark && owner)
+ for (RemarkId id : owner->findRemarks(related.query))
+ remark->addRelatedRemark(id);
+ return *this;
+}
+
InFlightRemark::~InFlightRemark() {
if (remark && owner)
owner->report(std::move(*remark));
@@ -174,6 +183,71 @@ InFlightRemark::~InFlightRemark() {
// Remark Engine
//===----------------------------------------------------------------------===//
+struct RemarkEngine::RemarkHistory {
+ struct Entry {
+ RemarkId id;
+ RemarkKind kind;
+ Location location;
+ std::string name, category, subCategory, function;
+ };
+ std::vector<Entry> entries;
+};
+
+void RemarkEngine::enableHistory(bool enable) {
+ std::lock_guard<std::mutex> lock(historyMutex);
+ if (enable) {
+ if (!history)
+ history = std::make_unique<RemarkHistory>();
+ } else {
+ history.reset();
+ }
+ historyEnabled.store(enable, std::memory_order_release);
+}
+
+void RemarkEngine::clearHistory() {
+ std::lock_guard<std::mutex> lock(historyMutex);
+ if (history)
+ history->entries.clear();
+}
+
+SmallVector<RemarkId>
+RemarkEngine::findRemarks(const RemarkQuery &query) const {
+ SmallVector<RemarkId> result;
+ std::lock_guard<std::mutex> lock(historyMutex);
+ if (!history)
+ return result;
+
+ auto matches = [](const auto &filter, const auto &value) {
+ return !filter || *filter == value;
+ };
+ for (const RemarkHistory::Entry &entry : history->entries)
+ if (matches(query.remarkName, entry.name) &&
+ matches(query.categoryName, entry.category) &&
+ matches(query.subCategoryName, entry.subCategory) &&
+ matches(query.functionName, entry.function) &&
+ matches(query.remarkKind, entry.kind) &&
+ matches(query.location, entry.location))
+ result.push_back(entry.id);
+ return result;
+}
+
+SmallVector<RemarkId>
+RemarkEngine::findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind) const {
+ RemarkQuery query;
+ if (!opts.remarkName.empty())
+ query = query.name(opts.remarkName);
+ if (!opts.categoryName.empty())
+ query = query.category(opts.categoryName);
+ if (!opts.subCategoryName.empty())
+ query = query.subCategory(opts.subCategoryName);
+ if (!opts.functionName.empty())
+ query = query.function(opts.functionName);
+ if (kind)
+ query = query.kind(*kind);
+ return findRemarks(query);
+}
+
template <typename RemarkT>
InFlightRemark RemarkEngine::makeRemark(Location loc, RemarkOpts opts) {
static_assert(std::is_base_of_v<Remark, RemarkT>,
@@ -264,6 +338,15 @@ void RemarkEngine::reportImpl(const Remark &remark) {
}
void RemarkEngine::report(const Remark &&remark) {
+ if (historyEnabled.load(std::memory_order_acquire)) {
+ std::lock_guard<std::mutex> lock(historyMutex);
+ if (history && remark.getId())
+ history->entries.push_back({remark.getId(), remark.getRemarkKind(),
+ remark.getLocation(), remark.remarkName,
+ remark.categoryName, remark.subCategoryName,
+ remark.functionName});
+ }
+ // Release the history lock before callbacks, which may themselves query it.
if (remarkEmittingPolicy)
remarkEmittingPolicy->reportRemark(remark);
}
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index ece76e628739fa..84cd2657792ffd 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -13,6 +13,7 @@
#include "mlir/IR/Remarks.h"
#include "mlir/Remark/RemarkStreamer.h"
#include "mlir/Support/TypeID.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/LLVMRemarkStreamer.h"
@@ -23,6 +24,7 @@
#include "llvm/Support/YAMLParser.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <functional>
#include <optional>
#include <vector>
@@ -795,6 +797,302 @@ TEST(Remark, TestRemarkLinking) {
EXPECT_THAT(errOut, HasSubstr("vectorized loop"));
}
+class QueryCallbackStreamer final
+ : public remark::detail::MLIRRemarkStreamerBase {
+public:
+ explicit QueryCallbackStreamer(
+ std::function<void(const remark::detail::Remark &)> callback)
+ : callback(std::move(callback)) {}
+
+ void streamOptimizationRemark(const remark::detail::Remark &remark) override {
+ callback(remark);
+ }
+
+private:
+ std::function<void(const remark::detail::Remark &)> callback;
+};
+
+class RemarkHistoryTest : public TestWithParam<bool> {
+protected:
+ struct Record {
+ remark::RemarkId id;
+ SmallVector<remark::RemarkId> related;
+ std::vector<std::string> serializedLinks;
+ };
+
+ void SetUp() override {
+ remark::RemarkCategories categories;
+ categories.passed = "(Opt.*)?";
+ categories.analysis = "(Opt.*)?";
+ std::unique_ptr<remark::detail::RemarkEmittingPolicyBase> policy;
+ if (GetParam())
+ policy = std::make_unique<remark::RemarkEmittingPolicyFinal>();
+ else
+ policy = std::make_unique<remark::RemarkEmittingPolicyAll>();
+ auto streamer = std::make_unique<QueryCallbackStreamer>([this](
+ const auto &r) {
+ Record record{r.getId(), {}, {}};
+ llvm::append_range(record.related, r.getRelatedRemarkIds());
+ for (const auto &arg : r.generateRemark().Args)
+ if (arg.Key == "RelatedTo")
+ record.serializedLinks.push_back(arg.Val.str());
+ records.push_back(std::move(record));
+ if (queryInCallback)
+ callbackHistory.push_back(engine().findRemarks(remark::RemarkQuery{}));
+ });
+ ASSERT_TRUE(succeeded(remark::enableOptimizationRemarks(
+ context, std::move(streamer), std::move(policy), categories)));
+ ASSERT_TRUE(engine().isPassedOptRemarkEnabled("Opt"));
+ ASSERT_TRUE(engine().isAnalysisOptRemarkEnabled("Opt"));
+ ASSERT_TRUE(engine().isPassedOptRemarkEnabled(""));
+ ASSERT_TRUE(engine().isAnalysisOptRemarkEnabled(""));
+ }
+
+ remark::detail::RemarkEngine &engine() { return *context.getRemarkEngine(); }
+
+ remark::RemarkId emit(Location loc, remark::RemarkOpts opts,
+ bool analysis = false) {
+ auto r = analysis ? remark::analysis(loc, opts) : remark::passed(loc, opts);
+ return r.getId();
+ }
+
+ void flush() { engine().getRemarkEmittingPolicy()->finalize(); }
+
+ std::vector<Record> records;
+ bool queryInCallback = false;
+ std::vector<SmallVector<remark::RemarkId>> callbackHistory;
+ // Keep callback state alive while the context drains its remark engine.
+ MLIRContext context;
+};
+
+TEST_P(RemarkHistoryTest, LifecycleAndReportOrder) {
+ Location loc = UnknownLoc::get(&context);
+ auto opts = remark::RemarkOpts::name("Candidate").category("Opt");
+ auto any = remark::RemarkQuery{};
+ emit(loc, opts);
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+
+ engine().enableHistory();
+ remark::RemarkId firstCreated, secondCreated;
+ {
+ auto pending = remark::passed(loc, opts);
+ firstCreated = pending.getId();
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ secondCreated = emit(loc, opts);
+ EXPECT_THAT(engine().findRemarks(any), ElementsAre(secondCreated));
+ }
+ EXPECT_LT(firstCreated.getValue(), secondCreated.getValue());
+ EXPECT_THAT(engine().findRemarks(any),
+ ElementsAre(secondCreated, firstCreated));
+
+ // FINAL may discard an earlier report, but history keeps both events.
+ flush();
+ engine().enableHistory();
+ EXPECT_THAT(engine().findRemarks(any),
+ ElementsAre(secondCreated, firstCreated));
+ engine().clearHistory();
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ auto afterClear = emit(loc, opts);
+ EXPECT_THAT(engine().findRemarks(any), ElementsAre(afterClear));
+ EXPECT_GT(afterClear.getValue(), secondCreated.getValue());
+
+ engine().enableHistory(false);
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ emit(loc, opts);
+ engine().clearHistory();
+ emit(loc, opts);
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ engine().enableHistory();
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ auto afterEnable = emit(loc, opts);
+ EXPECT_THAT(engine().findRemarks(any), ElementsAre(afterEnable));
+ EXPECT_GT(afterEnable.getValue(), afterClear.getValue());
+
+ EXPECT_FALSE(static_cast<bool>(
+ emit(loc, remark::RemarkOpts::name("Filtered").category("Unselected"))));
+ EXPECT_THAT(engine().findRemarks(any), ElementsAre(afterEnable));
+}
+
+TEST_P(RemarkHistoryTest, ExactFieldsAndConjunction) {
+ engine().enableHistory();
+ Location loc = FileLineColLoc::get(&context, "input.mlir", 7, 2);
+ Location otherLoc = NameLoc::get(StringAttr::get(&context, "wrapped"), loc);
+ auto opts = remark::RemarkOpts::name("Candidate")
+ .category("Opt")
+ .subCategory("Vector")
+ .function("kernel");
+ auto target = emit(loc, opts);
+ auto otherSub = emit(loc, opts.subCategory("Unroll"));
+ auto analysis = emit(loc, opts, /*analysis=*/true);
+ auto other = emit(otherLoc, remark::RemarkOpts::name("Other")
+ .category("OptOther")
+ .subCategory("Other")
+ .function("helper"));
+ auto empty = emit(UnknownLoc::get(&context), remark::RemarkOpts{});
+ auto any = remark::RemarkQuery{};
+
+ EXPECT_THAT(engine().findRemarks(any.name("Candidate")),
+ ElementsAre(target, otherSub, analysis));
+ EXPECT_THAT(engine().findRemarks(any.category("Opt")),
+ ElementsAre(target, otherSub, analysis));
+ EXPECT_THAT(engine().findRemarks(any.subCategory("Vector")),
+ ElementsAre(target, analysis));
+ EXPECT_THAT(engine().findRemarks(any.function("kernel")),
+ ElementsAre(target, otherSub, analysis));
+ EXPECT_THAT(engine().findRemarks(any.kind(remark::RemarkKind::RemarkPassed)),
+ ElementsAre(target, otherSub, other, empty));
+ EXPECT_THAT(engine().findRemarks(any.at(loc)),
+ ElementsAre(target, otherSub, analysis));
+ EXPECT_THAT(engine().findRemarks(any.at(otherLoc)), ElementsAre(other));
+
+ auto exact = any.name("Candidate")
+ .category("Opt")
+ .subCategory("Vector")
+ .function("kernel")
+ .kind(remark::RemarkKind::RemarkPassed)
+ .at(loc);
+ EXPECT_THAT(engine().findRemarks(exact), ElementsAre(target));
+ EXPECT_THAT(engine().findRemarks(exact.function("helper")), IsEmpty());
+ EXPECT_THAT(engine().findRemarks(any.category("Opt.*")), IsEmpty());
+ EXPECT_THAT(engine().findRemarks(any.category("Opt:Vector")), IsEmpty());
+ EXPECT_THAT(engine().findRemarks(any.name("")), ElementsAre(empty));
+ EXPECT_THAT(engine().findRemarks(any.category("")), ElementsAre(empty));
+ EXPECT_THAT(engine().findRemarks(any.subCategory("")), ElementsAre(empty));
+ EXPECT_THAT(engine().findRemarks(any.function("")), ElementsAre(empty));
+ EXPECT_THAT(engine().findRemarks(any.name("<unknown remark name>")),
+ IsEmpty());
+ EXPECT_THAT(engine().findRemarks(any.function("<unknown function>")),
+ IsEmpty());
+
+ // The existing options overload treats empty fields as wildcards.
+ EXPECT_THAT(engine().findRemarks(remark::RemarkOpts{}),
+ ElementsAre(target, otherSub, analysis, other, empty));
+ EXPECT_THAT(engine().findRemarks(opts, remark::RemarkKind::RemarkAnalysis),
+ ElementsAre(analysis));
+ EXPECT_THAT(engine().findRemarks(opts.relatedTo(other)),
+ ElementsAre(target, analysis));
+}
+
+TEST_P(RemarkHistoryTest, OwnsQueryAndHistoryStrings) {
+ engine().enableHistory();
+ Location loc = UnknownLoc::get(&context);
+ remark::RemarkQuery query;
+ remark::RemarkId target;
+ {
+ std::string name = "Candidate", category = "Opt", sub = "Vector";
+ std::string function = "kernel";
+ query =
+ query.name(name).category(category).subCategory(sub).function(function);
+ target = emit(loc, remark::RemarkOpts::name(name)
+ .category(category)
+ .subCategory(sub)
+ .function(function));
+ name.assign("Changed");
+ category.assign("Changed");
+ sub.assign("Changed");
+ function.assign("Changed");
+ }
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(target));
+ const auto original = query;
+ auto changed = original.name("Other");
+ EXPECT_THAT(engine().findRemarks(changed), IsEmpty());
+ EXPECT_THAT(engine().findRemarks(original), ElementsAre(target));
+}
+
+TEST_P(RemarkHistoryTest, QueryLinksSnapshotAndSerialization) {
+ engine().enableHistory();
+ Location loc = UnknownLoc::get(&context);
+ auto query = remark::RemarkQuery{}.kind(remark::RemarkKind::RemarkAnalysis);
+ auto first =
+ emit(loc, remark::RemarkOpts::name("First").category("Opt"), true);
+ auto second =
+ emit(loc, remark::RemarkOpts::name("Second").category("Opt"), true);
+ remark::RemarkId linked, later, pendingId, noMatches, copiedWrapper;
+ {
+ auto pending = remark::analysis(
+ loc, remark::RemarkOpts::name("Pending").category("Opt"));
+ pendingId = pending.getId();
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("Linked").category("Opt"));
+ linked = r.getId();
+ r << remark::relatedTo(query);
+ // This report occurs before Linked is reported, but after its query.
+ later =
+ emit(loc, remark::RemarkOpts::name("Later").category("Opt"), true);
+ }
+ }
+ {
+ const auto wrapper = remark::relatedTo(query);
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("CopiedWrapper").category("Opt"));
+ copiedWrapper = r.getId();
+ r << wrapper;
+ }
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("NoMatches").category("Opt"));
+ noMatches = r.getId();
+ r << remark::relatedTo(query.name("Absent"));
+ }
+ flush();
+
+ for (const auto &record : records) {
+ if (record.id == linked) {
+ EXPECT_THAT(record.related, ElementsAre(first, second));
+ EXPECT_THAT(record.serializedLinks,
+ ElementsAre(std::to_string(first.getValue()),
+ std::to_string(second.getValue())));
+ } else if (record.id == copiedWrapper) {
+ EXPECT_THAT(record.related, ElementsAre(first, second, later, pendingId));
+ } else if (record.id == noMatches) {
+ EXPECT_THAT(record.related, IsEmpty());
+ EXPECT_THAT(record.serializedLinks, IsEmpty());
+ }
+ }
+ EXPECT_THAT(records, Contains(Field(&Record::id, linked)));
+ EXPECT_THAT(records, Contains(Field(&Record::id, copiedWrapper)));
+ EXPECT_THAT(records, Contains(Field(&Record::id, noMatches)));
+}
+
+TEST_P(RemarkHistoryTest, QueryLinksDoNotEnableHistory) {
+ Location loc = UnknownLoc::get(&context);
+ auto query = remark::RemarkQuery{};
+ emit(loc, remark::RemarkOpts::name("First").category("Opt"));
+ remark::RemarkId linked;
+ {
+ auto r =
+ remark::passed(loc, remark::RemarkOpts::name("Linked").category("Opt"));
+ linked = r.getId();
+ auto wrapper = remark::relatedTo(query);
+ r << wrapper;
+ }
+ // Both filtered and ownerless remarks accept the lazy wrapper harmlessly.
+ remark::passed(loc, remark::RemarkOpts::name("Filtered").category("Other"))
+ << remark::relatedTo(query);
+ remark::detail::InFlightRemark{} << remark::relatedTo(query);
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+ engine().enableHistory();
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+ flush();
+ EXPECT_THAT(records, Contains(AllOf(Field(&Record::id, linked),
+ Field(&Record::related, IsEmpty()))));
+}
+
+TEST_P(RemarkHistoryTest, StreamerCanQueryCurrentReport) {
+ engine().enableHistory();
+ queryInCallback = true;
+ auto id = emit(UnknownLoc::get(&context),
+ remark::RemarkOpts::name("Candidate").category("Opt"));
+ flush();
+ EXPECT_THAT(callbackHistory, ElementsAre(ElementsAre(id)));
+}
+
+INSTANTIATE_TEST_SUITE_P(EmittingPolicies, RemarkHistoryTest, Bool(),
+ [](const TestParamInfo<bool> &info) {
+ return info.param ? "Final" : "All";
+ });
+
TEST(Remark, TestNestedLocation) {
MLIRContext context;
Location fileLoc = FileLineColLoc::get(&context, "test.mlir", 42, 7);
>From 231085ff624f9e4fd0319e92a3d396d7d64f8e93 Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Tue, 3 Nov 2026 10:49:52 +0100
Subject: [PATCH 2/2] [mlir] Record remark history after policy selection
---
mlir/docs/Remarks.md | 100 ++++-------
mlir/include/mlir/IR/Remarks.h | 134 +++++++++-----
mlir/lib/IR/Remarks.cpp | 121 +++++++++----
mlir/unittests/IR/RemarkTest.cpp | 291 +++++++++++++++++++++++++++++--
4 files changed, 494 insertions(+), 152 deletions(-)
diff --git a/mlir/docs/Remarks.md b/mlir/docs/Remarks.md
index 663eb1161e9ad8..0ff919588f08bc 100644
--- a/mlir/docs/Remarks.md
+++ b/mlir/docs/Remarks.md
@@ -253,86 +253,62 @@ if (engine->isRemarkEnabled(remark::RemarkKind::RemarkPassed, "Vectorizer"))
## Searching and Linking Remarks
-Remarks can refer to other remarks by ID using `RemarkOpts::relatedTo(id)`.
-When the producer does not have the ID, it can search previously reported
-remarks and link to the matches.
-
-First, enable history on an already configured engine:
+History is opt-in and records each emitted event ID once, in first-emission
+order, before the streamer callback. It retains IDs and identifying metadata,
+with interned strings, but not messages or arguments. `All` delivers reports
+immediately; `Final` makes them searchable only when finalized and excludes
+discarded events. Previously emitted targets remain searchable across batches.
+Enable history before emission: pending `Final` remarks can still be recorded
+when delivered, but earlier emissions and engine-filtered remarks are not recovered.
+
+`RemarkQuery` matches name, base category, subcategory, function, kind, and
+complete location, including wrappers. Specified fields use exact AND matching;
+unspecified fields match anything, and an empty string matches empty metadata.
+For example, with an already configured `All` engine and enabled categories:
```c++
auto *engine = context.getRemarkEngine();
engine->enableHistory();
-```
-
-History is disabled by default. Enabling it retains the IDs and identifying
-metadata of subsequent reports, without retaining their messages or arguments.
-It does not recover earlier reports or remarks excluded by the engine's
-category and kind filters. An in-flight remark becomes searchable when it is
-reported at the end of its lifetime.
-
-Use `RemarkQuery` to match a name, base category, subcategory, function, kind,
-or location. Specified fields are combined with AND and match exactly, without
-regular expressions. Unspecified fields match anything; an explicitly empty
-string matches empty metadata. Location matching compares the complete MLIR
-location, including wrappers, and does not identify a unique operation.
-
-```c++
auto analysisOpts = remark::RemarkOpts::name("VectorSizeInference")
.category("Vectorizer")
.function("kernel");
remark::analysis(loc, analysisOpts)
<< remark::reason("Unable to infer vector sizes");
-auto query = remark::RemarkQuery()
- .name("VectorSizeInference")
+auto query = remark::RemarkQuery::named("VectorSizeInference")
.category("Vectorizer")
.function("kernel")
.kind(remark::RemarkKind::RemarkAnalysis)
.at(loc);
-
-// Obtain the IDs explicitly, in report order.
+// Matching IDs are returned in first-emission order.
SmallVector<remark::RemarkId> matches = engine->findRemarks(query);
-
-// Or link all matching reports without handling their IDs.
remark::missed(loc, remark::RemarkOpts::name("Vectorization")
- .category("Vectorizer")
- .function("kernel"))
- << remark::reason("Cannot vectorize without vector sizes")
- << remark::relatedTo(query);
-```
-
-The helper resolves the query when inserted with `<<`. Later reports do not
-change the links. No matches, including when history is disabled, adds no
-links. Links use the existing repeated `RelatedTo` arguments in serialized
-output. Consumers must preserve repeated keys. A query match indicates a
-relationship, not causality; the producer is responsible for choosing relevant
-criteria. There is no automatic selection of the latest match or inference of
-whether earlier evidence is still valid for changed IR.
-
-Queries own their strings, so they can be retained and reused. A query with a
-location must not outlive that location's `MLIRContext`. History access is
-synchronized, and streamer callbacks may query the history, including the
-report being delivered. This does not synchronize the emitting policy or
-streamer. Report order follows insertion into history and can differ from ID
-creation order, including when remarks are emitted concurrently.
-
-History is independent of the output policy: both `All` and `Final` record
-every report, and finalizing a policy does not clear history. Use `All` when
-the output must contain every referenced event. `Final` may deduplicate a
-referenced report or emit it in an earlier batch, so finding its ID in history
-does not guarantee that the current output batch contains it.
-
-History grows with the number of reports until explicitly cleared, disabled,
-or the engine is destroyed. Clear it between compilations when reusing an
-engine:
-
-```c++
-engine->clearHistory(); // Keep recording subsequent reports.
-engine->enableHistory(false); // Discard history and stop recording.
+ .category("Vectorizer")
+ .function("kernel"))
+ << remark::relatedToLatest(query);
```
-Neither operation resets the ID counter or changes existing links. Repeated
-calls to `enableHistory()` preserve already recorded history.
+`relatedTo(query)` links all matches; `relatedToLatest(query)` links at most one,
+the most recent matching first-emission entry. Both resolve at `<<`; no match
+or disabled history adds no links, and later emissions do not change them.
+Known IDs can use `RemarkOpts::relatedTo(id)`. Serialized links repeat the
+`RelatedTo` key, so consumers must preserve duplicates. Matching does not establish
+causality, operation identity, or continued validity after IR changes.
+
+Queries own their criteria strings; queries with locations must not outlive the
+locations' `MLIRContext`.
+History access is synchronized, including queries from streamer callbacks that
+see the current event. This does not synchronize policies or streamers. With a
+parallel pass manager, cross-function query membership and ordering depend on
+timing; scope queries to a function and sequence producers or use explicit
+barriers. First-emission order can differ from ID creation order.
+
+Memory is retained until `clearHistory()`, `enableHistory(false)`, or engine
+destruction. Clearing keeps recording; disabling stops it. Neither resets IDs,
+changes existing links, nor cancels pending `Final` remarks. Repeated enabling
+preserves entries. Latest-match search scans backward and stops at its first
+match, but remains O(N) in the worst case. Repeated broad all-match queries can
+produce quadratic numbers of links.
***
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 6acbc722944ff4..49c5e1ff5abd70 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -13,6 +13,7 @@
#ifndef MLIR_IR_REMARKS_H
#define MLIR_IR_REMARKS_H
+#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/Remarks/Remark.h"
@@ -24,7 +25,7 @@
#include "mlir/IR/Value.h"
#include <atomic>
-#include <mutex>
+#include <utility>
namespace mlir::remark {
@@ -141,43 +142,60 @@ struct RemarkOpts {
inline RemarkOpts relatedTo(const detail::InFlightRemark &r) const;
};
-/// Exact-match criteria for searching previously reported remarks. Unspecified
+/// Exact-match criteria for searching previously emitted remarks. Unspecified
/// fields match any value; a specified empty string matches an empty field.
/// Strings are owned by the query. Locations belong to their MLIRContext and
/// must not outlive it. A location match does not imply operation identity.
class RemarkQuery {
public:
- RemarkQuery name(StringRef value) const {
- auto copy = *this;
- copy.remarkName = value.str();
- return copy;
+ /// Construct a query matching the given remark name.
+ [[nodiscard]] static RemarkQuery named(StringRef value) {
+ return RemarkQuery().name(value);
+ }
+
+ [[nodiscard]] RemarkQuery name(StringRef value) const & {
+ return RemarkQuery(*this).name(value);
+ }
+ [[nodiscard]] RemarkQuery name(StringRef value) && {
+ remarkName = value.str();
+ return std::move(*this);
}
/// Match the base category, without the subcategory.
- RemarkQuery category(StringRef value) const {
- auto copy = *this;
- copy.categoryName = value.str();
- return copy;
+ [[nodiscard]] RemarkQuery category(StringRef value) const & {
+ return RemarkQuery(*this).category(value);
}
- RemarkQuery subCategory(StringRef value) const {
- auto copy = *this;
- copy.subCategoryName = value.str();
- return copy;
+ [[nodiscard]] RemarkQuery category(StringRef value) && {
+ categoryName = value.str();
+ return std::move(*this);
}
- RemarkQuery function(StringRef value) const {
- auto copy = *this;
- copy.functionName = value.str();
- return copy;
+ [[nodiscard]] RemarkQuery subCategory(StringRef value) const & {
+ return RemarkQuery(*this).subCategory(value);
}
- RemarkQuery kind(RemarkKind value) const {
- auto copy = *this;
- copy.remarkKind = value;
- return copy;
+ [[nodiscard]] RemarkQuery subCategory(StringRef value) && {
+ subCategoryName = value.str();
+ return std::move(*this);
+ }
+ [[nodiscard]] RemarkQuery function(StringRef value) const & {
+ return RemarkQuery(*this).function(value);
+ }
+ [[nodiscard]] RemarkQuery function(StringRef value) && {
+ functionName = value.str();
+ return std::move(*this);
+ }
+ [[nodiscard]] RemarkQuery kind(RemarkKind value) const & {
+ return RemarkQuery(*this).kind(value);
+ }
+ [[nodiscard]] RemarkQuery kind(RemarkKind value) && {
+ remarkKind = value;
+ return std::move(*this);
}
/// Match the complete location, including any wrapping locations.
- RemarkQuery at(Location value) const {
- auto copy = *this;
- copy.location = value;
- return copy;
+ [[nodiscard]] RemarkQuery at(Location value) const & {
+ return RemarkQuery(*this).at(value);
+ }
+ [[nodiscard]] RemarkQuery at(Location value) && {
+ location = value;
+ return std::move(*this);
}
private:
@@ -194,8 +212,6 @@ namespace mlir::remark::detail {
// Remark Base Class
//===----------------------------------------------------------------------===//
class Remark {
- friend class RemarkEngine;
-
public:
Remark(RemarkKind remarkKind, DiagnosticSeverity severity, Location loc,
RemarkOpts opts)
@@ -267,8 +283,13 @@ class Remark {
return "<unknown function>";
}
+ /// Return the stored function name, which may be empty.
+ StringRef getRawFunction() const { return functionName; }
+
llvm::StringRef getCategoryName() const { return categoryName; }
+ llvm::StringRef getSubCategoryName() const { return subCategoryName; }
+
llvm::StringRef getCombinedCategoryName() const {
if (categoryName.empty() && subCategoryName.empty())
return {};
@@ -285,6 +306,9 @@ class Remark {
return remarkName;
}
+ /// Return the stored remark name, which may be empty.
+ StringRef getRawRemarkName() const { return remarkName; }
+
std::string getMsg() const;
ArrayRef<Arg> getArgs() const { return args; }
@@ -443,6 +467,7 @@ struct LazyTextBuild {
/// Resolves a query when inserted into an in-flight remark.
struct RelatedRemarkQuery {
RemarkQuery query;
+ bool latestOnly = false;
};
/// InFlightRemark is a RAII class that holds a reference to a Remark
@@ -535,6 +560,14 @@ class RemarkEmittingPolicyBase {
virtual void reportRemark(const Remark &remark) = 0;
virtual void finalize() = 0;
+
+ /// Legacy policy-specific lookup hook. Engine history queries use the
+ /// records actually emitted by the policy, independently of this hook.
+ virtual SmallVector<RemarkId>
+ findRemarks(const RemarkOpts &opts,
+ std::optional<RemarkKind> kind = std::nullopt) const {
+ return {};
+ }
};
//===----------------------------------------------------------------------===//
@@ -543,6 +576,8 @@ class RemarkEmittingPolicyBase {
class RemarkEngine {
private:
+ friend class InFlightRemark;
+
/// Regex that filters missed optimization remarks: only matching one are
/// reported.
std::optional<llvm::Regex> missFilter;
@@ -561,12 +596,17 @@ class RemarkEngine {
/// Atomic counter for generating unique remark IDs.
std::atomic<uint64_t> nextRemarkId{1};
- /// Search history is independent of the emitting policy. Only metadata is
- /// retained, and only after explicitly enabling history.
+ /// History records metadata for remarks selected for emission by the policy.
+ /// Its mutex has a stable lifetime; retained storage is allocated on demand.
struct RemarkHistory;
std::unique_ptr<RemarkHistory> history;
std::atomic<bool> historyEnabled{false};
- mutable std::mutex historyMutex;
+
+ /// Visit matches while holding the history lock. Only internal callbacks that
+ /// do not reenter the engine may be used here.
+ void forEachMatch(const RemarkQuery &query,
+ llvm::function_ref<void(RemarkId)> callback,
+ bool latestOnly = false) const;
/// Emit a remark using the given maker function, which should return
/// a Remark instance. The remark will be emitted using the main
@@ -653,8 +693,8 @@ class RemarkEngine {
// Remark Linking - query previously emitted remarks
//===--------------------------------------------------------------------===//
- /// Enable recording metadata for subsequent reports. Repeated enabling
- /// preserves history. Disabling discards history. No past events are
+ /// Enable recording metadata for subsequent policy emissions. Repeated
+ /// enabling preserves history. Disabling discards history. No past events are
/// recovered. History is retained until cleared, disabled, or the engine is
/// destroyed; callers reusing an engine across compilations should clear it
/// between runs.
@@ -664,12 +704,17 @@ class RemarkEngine {
/// Existing remark IDs and links are unaffected.
void clearHistory();
- /// Find all recorded remarks matching every specified criterion, in report
- /// order. Queries see a snapshot of history, including reports not yet
- /// emitted by a deferred policy. In-flight and filtered-out remarks are not
- /// recorded. Returns empty when history is disabled. Policy finalization does
- /// not clear history. History access is synchronized; this does not make
- /// emitting policies or streamers thread-safe.
+ /// Find distinct emitted remarks matching every specified criterion, in
+ /// first-emission order. Pending, engine-filtered and policy-discarded
+ /// remarks are absent. Returns empty when history is disabled. Policy
+ /// finalization may add entries but does not clear history. A callback can
+ /// query the event currently being delivered. History access is synchronized;
+ /// this does not make emitting policies or streamers thread-safe.
+ ///
+ /// Cross-function queries during parallel pass execution have
+ /// timing-dependent results and ordering. Scope queries with function() and
+ /// sequence producers before consumers when deterministic relationships are
+ /// required.
SmallVector<RemarkId> findRemarks(const RemarkQuery &query) const;
/// Search using nonempty option fields as exact-match criteria. The related
@@ -721,13 +766,20 @@ mlir::remark::RemarkOpts::relatedTo(const detail::InFlightRemark &r) const {
namespace mlir::remark {
/// Link to all remarks matching the query when this helper is inserted into an
-/// in-flight remark. History must have been enabled before reporting the target
-/// remarks. No matches (including disabled history) adds no links. Matching
+/// in-flight remark. History must have been enabled before the policy emitted
+/// the remarks. No matches (including disabled history) adds no links. Matching
/// indicates a relationship, not causality or operation identity.
inline detail::RelatedRemarkQuery relatedTo(RemarkQuery query) {
return {std::move(query)};
}
+/// Link only the latest match in first-emission order. The reverse search stops
+/// at the first match and adds at most one RelatedTo argument. Repeated
+/// delivery of an existing event does not refresh its position in history.
+inline detail::RelatedRemarkQuery relatedToLatest(RemarkQuery query) {
+ return {std::move(query), true};
+}
+
//===----------------------------------------------------------------------===//
// Remark Emitting Policies
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp
index e3f90ebf617b89..d912d35020e31f 100644
--- a/mlir/lib/IR/Remarks.cpp
+++ b/mlir/lib/IR/Remarks.cpp
@@ -14,7 +14,9 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/StringSaver.h"
+#include <mutex>
#include <vector>
using namespace mlir::remark::detail;
@@ -168,8 +170,9 @@ llvm::remarks::Remark Remark::generateRemark() const {
InFlightRemark &InFlightRemark::operator<<(const RelatedRemarkQuery &related) {
if (remark && owner)
- for (RemarkId id : owner->findRemarks(related.query))
- remark->addRelatedRemark(id);
+ owner->forEachMatch(
+ related.query, [this](RemarkId id) { remark->addRelatedRemark(id); },
+ related.latestOnly);
return *this;
}
@@ -188,46 +191,90 @@ struct RemarkEngine::RemarkHistory {
RemarkId id;
RemarkKind kind;
Location location;
- std::string name, category, subCategory, function;
+ StringRef name, category, subCategory, function;
+
+ static Entry from(const Remark &remark, llvm::UniqueStringSaver &strings) {
+ return {remark.getId(),
+ remark.getRemarkKind(),
+ remark.getLocation(),
+ strings.save(remark.getRawRemarkName()),
+ strings.save(remark.getCategoryName()),
+ strings.save(remark.getSubCategoryName()),
+ strings.save(remark.getRawFunction())};
+ }
+ };
+
+ struct Storage {
+ llvm::BumpPtrAllocator allocator;
+ llvm::UniqueStringSaver strings{allocator};
+ std::vector<Entry> entries;
+ llvm::DenseSet<uint64_t> emittedIds;
};
- std::vector<Entry> entries;
+
+ // Keep the mutex alive for the engine's lifetime so disabling history cannot
+ // destroy it while another thread is about to lock it. Storage is opt-in.
+ std::mutex mutex;
+ std::unique_ptr<Storage> storage;
};
void RemarkEngine::enableHistory(bool enable) {
- std::lock_guard<std::mutex> lock(historyMutex);
+ std::scoped_lock lock(history->mutex);
if (enable) {
- if (!history)
- history = std::make_unique<RemarkHistory>();
+ if (!history->storage)
+ history->storage = std::make_unique<RemarkHistory::Storage>();
} else {
- history.reset();
+ history->storage.reset();
}
historyEnabled.store(enable, std::memory_order_release);
}
void RemarkEngine::clearHistory() {
- std::lock_guard<std::mutex> lock(historyMutex);
- if (history)
- history->entries.clear();
+ if (!historyEnabled.load(std::memory_order_acquire))
+ return;
+ std::scoped_lock lock(history->mutex);
+ if (history->storage)
+ history->storage = std::make_unique<RemarkHistory::Storage>();
}
-SmallVector<RemarkId>
-RemarkEngine::findRemarks(const RemarkQuery &query) const {
- SmallVector<RemarkId> result;
- std::lock_guard<std::mutex> lock(historyMutex);
- if (!history)
- return result;
+void RemarkEngine::forEachMatch(const RemarkQuery &query,
+ llvm::function_ref<void(RemarkId)> callback,
+ bool latestOnly) const {
+ if (!historyEnabled.load(std::memory_order_acquire))
+ return;
+ std::scoped_lock lock(history->mutex);
+ if (!history->storage)
+ return;
auto matches = [](const auto &filter, const auto &value) {
return !filter || *filter == value;
};
- for (const RemarkHistory::Entry &entry : history->entries)
- if (matches(query.remarkName, entry.name) &&
- matches(query.categoryName, entry.category) &&
- matches(query.subCategoryName, entry.subCategory) &&
- matches(query.functionName, entry.function) &&
- matches(query.remarkKind, entry.kind) &&
- matches(query.location, entry.location))
- result.push_back(entry.id);
+ auto matchesQuery = [&](const RemarkHistory::Entry &entry) {
+ return matches(query.remarkName, entry.name) &&
+ matches(query.categoryName, entry.category) &&
+ matches(query.subCategoryName, entry.subCategory) &&
+ matches(query.functionName, entry.function) &&
+ matches(query.remarkKind, entry.kind) &&
+ matches(query.location, entry.location);
+ };
+ const auto &entries = history->storage->entries;
+ if (latestOnly) {
+ for (const RemarkHistory::Entry &entry : llvm::reverse(entries)) {
+ if (!matchesQuery(entry))
+ continue;
+ callback(entry.id);
+ break;
+ }
+ return;
+ }
+ for (const RemarkHistory::Entry &entry : entries)
+ if (matchesQuery(entry))
+ callback(entry.id);
+}
+
+SmallVector<RemarkId>
+RemarkEngine::findRemarks(const RemarkQuery &query) const {
+ SmallVector<RemarkId> result;
+ forEachMatch(query, [&](RemarkId id) { result.push_back(id); });
return result;
}
@@ -327,6 +374,18 @@ InFlightRemark RemarkEngine::emitOptimizationRemarkAnalysis(Location loc,
//===----------------------------------------------------------------------===//
void RemarkEngine::reportImpl(const Remark &remark) {
+ if (historyEnabled.load(std::memory_order_acquire)) {
+ std::scoped_lock lock(history->mutex);
+ if (history->storage && remark.getId()) {
+ auto &storage = *history->storage;
+ // FINAL can emit a shared related remark more than once. Keep one history
+ // entry per event so a query does not produce duplicate links.
+ if (storage.emittedIds.insert(remark.getId().getValue()).second)
+ storage.entries.push_back(
+ RemarkHistory::Entry::from(remark, storage.strings));
+ }
+ }
+ // Release the history lock before callbacks, which may themselves query it.
// Stream the remark
if (remarkStreamer) {
remarkStreamer->streamOptimizationRemark(remark);
@@ -338,15 +397,6 @@ void RemarkEngine::reportImpl(const Remark &remark) {
}
void RemarkEngine::report(const Remark &&remark) {
- if (historyEnabled.load(std::memory_order_acquire)) {
- std::lock_guard<std::mutex> lock(historyMutex);
- if (history && remark.getId())
- history->entries.push_back({remark.getId(), remark.getRemarkKind(),
- remark.getLocation(), remark.remarkName,
- remark.categoryName, remark.subCategoryName,
- remark.functionName});
- }
- // Release the history lock before callbacks, which may themselves query it.
if (remarkEmittingPolicy)
remarkEmittingPolicy->reportRemark(remark);
}
@@ -413,7 +463,8 @@ buildFilter(const mlir::remark::RemarkCategories &cats,
RemarkEngine::RemarkEngine(bool printAsEmitRemarks,
const RemarkCategories &cats)
- : printAsEmitRemarks(printAsEmitRemarks) {
+ : printAsEmitRemarks(printAsEmitRemarks),
+ history(std::make_unique<RemarkHistory>()) {
if (cats.passed)
passedFilter = buildFilter(cats, cats.passed);
if (cats.missed)
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index 84cd2657792ffd..9b1c4240165411 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -858,6 +858,17 @@ class RemarkHistoryTest : public TestWithParam<bool> {
void flush() { engine().getRemarkEmittingPolicy()->finalize(); }
+ void expectLinks(remark::RemarkId id, ArrayRef<remark::RemarkId> expected) {
+ std::vector<std::string> serialized;
+ for (remark::RemarkId related : expected)
+ serialized.push_back(std::to_string(related.getValue()));
+ EXPECT_THAT(records, Contains(AllOf(Field(&Record::id, id),
+ Field(&Record::related,
+ ElementsAreArray(expected)),
+ Field(&Record::serializedLinks,
+ ElementsAreArray(serialized)))));
+ }
+
std::vector<Record> records;
bool queryInCallback = false;
std::vector<SmallVector<remark::RemarkId>> callbackHistory;
@@ -865,11 +876,12 @@ class RemarkHistoryTest : public TestWithParam<bool> {
MLIRContext context;
};
-TEST_P(RemarkHistoryTest, LifecycleAndReportOrder) {
+TEST_P(RemarkHistoryTest, LifecycleAndEmissionOrder) {
Location loc = UnknownLoc::get(&context);
auto opts = remark::RemarkOpts::name("Candidate").category("Opt");
auto any = remark::RemarkQuery{};
emit(loc, opts);
+ flush();
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
engine().enableHistory();
@@ -878,33 +890,42 @@ TEST_P(RemarkHistoryTest, LifecycleAndReportOrder) {
auto pending = remark::passed(loc, opts);
firstCreated = pending.getId();
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
- secondCreated = emit(loc, opts);
- EXPECT_THAT(engine().findRemarks(any), ElementsAre(secondCreated));
+ secondCreated =
+ emit(loc, remark::RemarkOpts::name("Second").category("Opt"));
+ if (GetParam())
+ EXPECT_THAT(engine().findRemarks(any), IsEmpty());
+ else
+ EXPECT_THAT(engine().findRemarks(any), ElementsAre(secondCreated));
}
EXPECT_LT(firstCreated.getValue(), secondCreated.getValue());
- EXPECT_THAT(engine().findRemarks(any),
- ElementsAre(secondCreated, firstCreated));
-
- // FINAL may discard an earlier report, but history keeps both events.
+ // ALL emits immediately; FINAL emits these independent records in ID order.
flush();
engine().enableHistory();
- EXPECT_THAT(engine().findRemarks(any),
- ElementsAre(secondCreated, firstCreated));
+ if (GetParam())
+ EXPECT_THAT(engine().findRemarks(any),
+ ElementsAre(firstCreated, secondCreated));
+ else
+ EXPECT_THAT(engine().findRemarks(any),
+ ElementsAre(secondCreated, firstCreated));
engine().clearHistory();
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
auto afterClear = emit(loc, opts);
+ flush();
EXPECT_THAT(engine().findRemarks(any), ElementsAre(afterClear));
EXPECT_GT(afterClear.getValue(), secondCreated.getValue());
engine().enableHistory(false);
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
emit(loc, opts);
+ flush();
engine().clearHistory();
emit(loc, opts);
+ flush();
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
engine().enableHistory();
EXPECT_THAT(engine().findRemarks(any), IsEmpty());
auto afterEnable = emit(loc, opts);
+ flush();
EXPECT_THAT(engine().findRemarks(any), ElementsAre(afterEnable));
EXPECT_GT(afterEnable.getValue(), afterClear.getValue());
@@ -930,6 +951,7 @@ TEST_P(RemarkHistoryTest, ExactFieldsAndConjunction) {
.function("helper"));
auto empty = emit(UnknownLoc::get(&context), remark::RemarkOpts{});
auto any = remark::RemarkQuery{};
+ flush();
EXPECT_THAT(engine().findRemarks(any.name("Candidate")),
ElementsAre(target, otherSub, analysis));
@@ -981,8 +1003,10 @@ TEST_P(RemarkHistoryTest, OwnsQueryAndHistoryStrings) {
{
std::string name = "Candidate", category = "Opt", sub = "Vector";
std::string function = "kernel";
- query =
- query.name(name).category(category).subCategory(sub).function(function);
+ query = remark::RemarkQuery::named(name)
+ .category(category)
+ .subCategory(sub)
+ .function(function);
target = emit(loc, remark::RemarkOpts::name(name)
.category(category)
.subCategory(sub)
@@ -992,11 +1016,32 @@ TEST_P(RemarkHistoryTest, OwnsQueryAndHistoryStrings) {
sub.assign("Changed");
function.assign("Changed");
}
+ flush();
EXPECT_THAT(engine().findRemarks(query), ElementsAre(target));
const auto original = query;
auto changed = original.name("Other");
EXPECT_THAT(engine().findRemarks(changed), IsEmpty());
EXPECT_THAT(engine().findRemarks(original), ElementsAre(target));
+
+ // Saved queries and copied IDs remain usable after the interned pool resets.
+ auto savedIds = engine().findRemarks(query);
+ for (bool disable : {false, true}) {
+ if (disable) {
+ engine().enableHistory(false);
+ engine().enableHistory();
+ } else {
+ engine().clearHistory();
+ }
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+ auto next = emit(loc, remark::RemarkOpts::name("Candidate")
+ .category("Opt")
+ .subCategory("Vector")
+ .function("kernel"));
+ flush();
+ EXPECT_THAT(engine().findRemarks(original), ElementsAre(next));
+ EXPECT_GT(next.getValue(), target.getValue());
+ EXPECT_THAT(savedIds, ElementsAre(target));
+ }
}
TEST_P(RemarkHistoryTest, QueryLinksSnapshotAndSerialization) {
@@ -1007,6 +1052,7 @@ TEST_P(RemarkHistoryTest, QueryLinksSnapshotAndSerialization) {
emit(loc, remark::RemarkOpts::name("First").category("Opt"), true);
auto second =
emit(loc, remark::RemarkOpts::name("Second").category("Opt"), true);
+ flush();
remark::RemarkId linked, later, pendingId, noMatches, copiedWrapper;
{
auto pending = remark::analysis(
@@ -1022,6 +1068,7 @@ TEST_P(RemarkHistoryTest, QueryLinksSnapshotAndSerialization) {
emit(loc, remark::RemarkOpts::name("Later").category("Opt"), true);
}
}
+ flush();
{
const auto wrapper = remark::relatedTo(query);
auto r = remark::passed(
@@ -1044,7 +1091,12 @@ TEST_P(RemarkHistoryTest, QueryLinksSnapshotAndSerialization) {
ElementsAre(std::to_string(first.getValue()),
std::to_string(second.getValue())));
} else if (record.id == copiedWrapper) {
- EXPECT_THAT(record.related, ElementsAre(first, second, later, pendingId));
+ if (GetParam())
+ EXPECT_THAT(record.related,
+ ElementsAre(first, second, pendingId, later));
+ else
+ EXPECT_THAT(record.related,
+ ElementsAre(first, second, later, pendingId));
} else if (record.id == noMatches) {
EXPECT_THAT(record.related, IsEmpty());
EXPECT_THAT(record.serializedLinks, IsEmpty());
@@ -1059,7 +1111,7 @@ TEST_P(RemarkHistoryTest, QueryLinksDoNotEnableHistory) {
Location loc = UnknownLoc::get(&context);
auto query = remark::RemarkQuery{};
emit(loc, remark::RemarkOpts::name("First").category("Opt"));
- remark::RemarkId linked;
+ remark::RemarkId linked, latest;
{
auto r =
remark::passed(loc, remark::RemarkOpts::name("Linked").category("Opt"));
@@ -1067,16 +1119,205 @@ TEST_P(RemarkHistoryTest, QueryLinksDoNotEnableHistory) {
auto wrapper = remark::relatedTo(query);
r << wrapper;
}
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("LatestDisabled").category("Opt"));
+ latest = r.getId();
+ r << remark::relatedToLatest(query);
+ }
// Both filtered and ownerless remarks accept the lazy wrapper harmlessly.
remark::passed(loc, remark::RemarkOpts::name("Filtered").category("Other"))
<< remark::relatedTo(query);
remark::detail::InFlightRemark{} << remark::relatedTo(query);
+ remark::detail::InFlightRemark{} << remark::relatedToLatest(query);
EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+ flush();
engine().enableHistory();
EXPECT_THAT(engine().findRemarks(query), IsEmpty());
- flush();
EXPECT_THAT(records, Contains(AllOf(Field(&Record::id, linked),
Field(&Record::related, IsEmpty()))));
+ expectLinks(latest, {});
+}
+
+TEST_P(RemarkHistoryTest, HistoryEnableAndClearUseEmissionBoundary) {
+ Location loc = UnknownLoc::get(&context);
+ auto query = remark::RemarkQuery{};
+ auto beforeEnable =
+ emit(loc, remark::RemarkOpts::name("BeforeEnable").category("Opt"));
+ engine().enableHistory();
+ flush();
+ if (GetParam())
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(beforeEnable));
+ else
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+
+ auto beforeClear =
+ emit(loc, remark::RemarkOpts::name("BeforeClear").category("Opt"));
+ engine().clearHistory();
+ flush();
+ if (GetParam())
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(beforeClear));
+ else
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+
+ emit(loc, remark::RemarkOpts::name("BeforeDisable").category("Opt"));
+ engine().enableHistory(false);
+ flush();
+ engine().enableHistory();
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+}
+
+TEST_P(RemarkHistoryTest, SameKeyQueriesLinkOnlyEmittedTargets) {
+ engine().enableHistory();
+ Location loc = UnknownLoc::get(&context);
+ auto opts = remark::RemarkOpts::name("Candidate").category("Opt");
+ auto query = remark::RemarkQuery::named("Candidate").category("Opt");
+ auto first = emit(loc, opts);
+ auto second = emit(loc, opts);
+ if (GetParam())
+ EXPECT_THAT(engine().findRemarks(query), IsEmpty());
+ else
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(first, second));
+
+ remark::RemarkId beforeFlush;
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("BeforeFlush").category("Opt"));
+ beforeFlush = r.getId();
+ r << remark::relatedTo(query);
+ }
+ flush();
+ if (GetParam()) {
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(second));
+ EXPECT_THAT(records, Not(Contains(Field(&Record::id, first))));
+ expectLinks(beforeFlush, {});
+ } else {
+ EXPECT_THAT(engine().findRemarks(query), ElementsAre(first, second));
+ expectLinks(beforeFlush, {first, second});
+ }
+
+ // The same identity can link to an earlier batch without suppressing it.
+ remark::RemarkId sameKey;
+ {
+ auto r = remark::passed(loc, opts);
+ sameKey = r.getId();
+ r << remark::relatedToLatest(query);
+ }
+ flush();
+ expectLinks(sameKey, {second});
+
+ // A pending FINAL replacement must not become a dangling query target.
+ auto replaced = emit(loc, opts);
+ remark::RemarkId consumer;
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("Consumer").category("Opt"));
+ consumer = r.getId();
+ r << remark::relatedToLatest(query);
+ emit(loc, opts);
+ }
+ flush();
+ expectLinks(consumer, {GetParam() ? sameKey : replaced});
+ if (GetParam())
+ EXPECT_THAT(records, Not(Contains(Field(&Record::id, replaced))));
+ for (const Record &record : records)
+ for (remark::RemarkId related : record.related)
+ EXPECT_THAT(records, Contains(Field(&Record::id, related)));
+}
+
+TEST_P(RemarkHistoryTest, LatestLinkUsesEmissionOrderAndInsertionTime) {
+ engine().enableHistory();
+ Location loc = UnknownLoc::get(&context);
+ auto query = remark::RemarkQuery{}.kind(remark::RemarkKind::RemarkAnalysis);
+ remark::RemarkId firstCreated, secondCreated;
+ {
+ auto pending = remark::analysis(
+ loc, remark::RemarkOpts::name("First").category("Opt"));
+ firstCreated = pending.getId();
+ secondCreated =
+ emit(loc, remark::RemarkOpts::name("Second").category("Opt"), true);
+ }
+ flush();
+ ASSERT_LT(firstCreated.getValue(), secondCreated.getValue());
+ auto latest = GetParam() ? secondCreated : firstCreated;
+ remark::RemarkId linked;
+ {
+ const auto wrapper = remark::relatedToLatest(query);
+ auto r =
+ remark::passed(loc, remark::RemarkOpts::name("Linked").category("Opt"));
+ linked = r.getId();
+ r << wrapper;
+ emit(loc, remark::RemarkOpts::name("Later").category("Opt"), true);
+ // Even a later emission before this remark is reported cannot retarget it.
+ flush();
+ }
+ flush();
+ expectLinks(linked, {latest});
+
+ const auto savedWrapper = remark::relatedToLatest(query);
+ engine().clearHistory();
+ remark::RemarkId noMatch;
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("NoMatch").category("Opt"));
+ noMatch = r.getId();
+ r << savedWrapper;
+ }
+ flush();
+ expectLinks(noMatch, {});
+
+ auto fresh =
+ emit(loc, remark::RemarkOpts::name("Fresh").category("Opt"), true);
+ flush();
+ remark::RemarkId afterReset;
+ {
+ auto r = remark::passed(
+ loc, remark::RemarkOpts::name("AfterReset").category("Opt"));
+ afterReset = r.getId();
+ r << savedWrapper;
+ }
+ flush();
+ expectLinks(afterReset, {fresh});
+}
+
+TEST_P(RemarkHistoryTest, RepeatedDeliveryRetainsFirstEmissionOrder) {
+ engine().enableHistory();
+ Location loc = UnknownLoc::get(&context);
+ auto child =
+ emit(loc, remark::RemarkOpts::name("Child").category("Opt"), true);
+ auto firstParent = emit(
+ loc,
+ remark::RemarkOpts::name("FirstParent").category("Opt").relatedTo(child));
+ auto secondParent = emit(
+ loc,
+ remark::RemarkOpts::name("SecondParent").category("Opt").relatedTo(child),
+ true);
+ flush();
+ auto query = remark::RemarkQuery{};
+ if (GetParam()) {
+ EXPECT_THAT(engine().findRemarks(query),
+ ElementsAre(firstParent, child, secondParent));
+ EXPECT_EQ(
+ llvm::count_if(records, [&](const Record &r) { return r.id == child; }),
+ 2);
+ } else {
+ EXPECT_THAT(engine().findRemarks(query),
+ ElementsAre(child, firstParent, secondParent));
+ }
+
+ // FINAL delivered Child again after SecondParent; it is still the older
+ // match.
+ auto analyses = query.kind(remark::RemarkKind::RemarkAnalysis);
+ EXPECT_THAT(engine().findRemarks(analyses), ElementsAre(child, secondParent));
+ remark::RemarkId linked;
+ {
+ auto r =
+ remark::passed(loc, remark::RemarkOpts::name("Latest").category("Opt"));
+ linked = r.getId();
+ r << remark::relatedToLatest(analyses);
+ }
+ flush();
+ expectLinks(linked, {secondParent});
}
TEST_P(RemarkHistoryTest, StreamerCanQueryCurrentReport) {
@@ -1093,6 +1334,28 @@ INSTANTIATE_TEST_SUITE_P(EmittingPolicies, RemarkHistoryTest, Bool(),
return info.param ? "Final" : "All";
});
+TEST(Remark, RawMetadataAccessorsPreserveEmptyFields) {
+ MLIRContext context;
+ Location loc = UnknownLoc::get(&context);
+ remark::detail::OptRemarkPass empty(loc, remark::RemarkOpts{});
+ EXPECT_TRUE(empty.getRawRemarkName().empty());
+ EXPECT_TRUE(empty.getRawFunction().empty());
+ EXPECT_TRUE(empty.getSubCategoryName().empty());
+ EXPECT_EQ(empty.getRemarkName(), "<unknown remark name>");
+ EXPECT_EQ(empty.getFunction(), "<unknown function>");
+
+ remark::detail::OptRemarkPass populated(loc,
+ remark::RemarkOpts::name("Candidate")
+ .category("Opt")
+ .subCategory("Vector")
+ .function("kernel"));
+ EXPECT_EQ(populated.getRawRemarkName(), "Candidate");
+ EXPECT_EQ(populated.getRawFunction(), "kernel");
+ EXPECT_EQ(populated.getCategoryName(), "Opt");
+ EXPECT_EQ(populated.getSubCategoryName(), "Vector");
+ EXPECT_EQ(populated.getCombinedCategoryName(), "Opt:Vector");
+}
+
TEST(Remark, TestNestedLocation) {
MLIRContext context;
Location fileLoc = FileLineColLoc::get(&context, "test.mlir", 42, 7);
More information about the Mlir-commits
mailing list