[Mlir-commits] [mlir] [mlir] Add searchable remark history and query-based links (PR #237762)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sun Nov 1 11:39:29 PST 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: Guray Ozen (grypp)
<details>
<summary>Changes</summary>
Remark producers currently need to retain explicit `RemarkId`s to link a later decision to earlier evidence. This adds opt-in, engine-owned history and exact-match queries so producers can find previously reported remarks and link all matches without handling their IDs.
```cpp
engine->enableHistory(); // Before reporting the evidence to be searched.
// After the analysis remarks have been reported:
auto query = remark::RemarkQuery()
.name("VectorSizeInference")
.category("Vectorizer")
.function("kernel")
.at(loc);
remark::missed(loc, opts) << remark::relatedTo(query);
```
Queries support name, base category, subcategory, function, kind, and complete MLIR location. Specified criteria are combined with AND; results follow report order. `relatedTo(query)` resolves at insertion time and uses the existing repeated `RelatedTo` arguments for serialization. The existing `findRemarks(RemarkOpts, kind)` overload is also implemented.
History stores owned identifying metadata independently of the emitting policy. It records subsequent reports until cleared, disabled, or the engine is destroyed; filtered and still-in-flight remarks are absent. History locking is released before callbacks, allowing streamers to query the report being delivered.
Use `ALL` when serialized output must contain every referenced event: `FINAL` can discard a target that remains searchable in history. Query matches express relatedness; location matching does not establish operation identity or causality. This change adds the C++ API and documentation.
Validation in an isolated Debug build with assertions:
- `MLIRIRTests`: 189 passed, including 12 new cases covering both emitting policies.
- `MLIRRemarkTests`: 11 passed.
- New coverage includes exact and empty-field matching, owned strings, history lifecycle, report order, immediate multi-result links, serialized link arguments, and reentrant callback queries.
---
Patch is 28.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/237762.diff
4 Files Affected:
- (modified) mlir/docs/Remarks.md (+85)
- (modified) mlir/include/mlir/IR/Remarks.h (+96-17)
- (modified) mlir/lib/IR/Remarks.cpp (+83)
- (modified) mlir/unittests/IR/RemarkTest.cpp (+298)
``````````diff
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());
+
+ EX...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/237762
More information about the Mlir-commits
mailing list