[Mlir-commits] [mlir] [MLIR] Add option to postpone remark emission (PR #157434)
Guray Ozen
llvmlistbot at llvm.org
Tue Sep 16 01:49:33 PDT 2025
https://github.com/grypp updated https://github.com/llvm/llvm-project/pull/157434
>From 4bd61633a75370a25e405a5f1e1c35cbb0a600be Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Mon, 8 Sep 2025 13:58:13 +0200
Subject: [PATCH 1/6] [MLIR] Add option to postpone remark emission
By default, `InFlightRemark` are emitted as soon as their scope ends.
This PR adds a mode to postpone emission until the end of compilation, so remarks are processed at once.
This feature will allow later passes to drop, rewrite, or aggregate remarks before they are printed/streamed. This will be done in follow-up PRs.
---
mlir/include/mlir/IR/Remarks.h | 27 +++++++---
mlir/lib/IR/Remarks.cpp | 16 +++++-
mlir/unittests/IR/RemarkTest.cpp | 87 ++++++++++++++++++++++++++++++++
3 files changed, 123 insertions(+), 7 deletions(-)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 20e84ec83cd01..94fce3adb32cf 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -60,22 +60,27 @@ struct RemarkOpts {
StringRef categoryName; // Category name (subject to regex filtering)
StringRef subCategoryName; // Subcategory name
StringRef functionName; // Function name if available
+ bool postponed = false; // Postpone showing the remark
// Construct RemarkOpts from a remark name.
static constexpr RemarkOpts name(StringRef n) {
- return RemarkOpts{n, {}, {}, {}};
+ return RemarkOpts{n, {}, {}, {}, false};
}
/// Return a copy with the category set.
constexpr RemarkOpts category(StringRef v) const {
- return {remarkName, v, subCategoryName, functionName};
+ return {remarkName, v, subCategoryName, functionName, postponed};
}
/// Return a copy with the subcategory set.
constexpr RemarkOpts subCategory(StringRef v) const {
- return {remarkName, categoryName, v, functionName};
+ return {remarkName, categoryName, v, functionName, postponed};
}
/// Return a copy with the function name set.
constexpr RemarkOpts function(StringRef v) const {
- return {remarkName, categoryName, subCategoryName, v};
+ return {remarkName, categoryName, subCategoryName, v, postponed};
+ }
+ /// Return a copy with the function name set.
+ constexpr RemarkOpts postpone() const {
+ return {remarkName, categoryName, subCategoryName, functionName, true};
}
};
@@ -92,12 +97,13 @@ class Remark {
RemarkOpts opts)
: remarkKind(remarkKind), functionName(opts.functionName), loc(loc),
categoryName(opts.categoryName), subCategoryName(opts.subCategoryName),
- remarkName(opts.remarkName) {
+ remarkName(opts.remarkName), postponed(opts.postponed) {
if (!categoryName.empty() && !subCategoryName.empty()) {
(llvm::Twine(categoryName) + ":" + subCategoryName)
.toStringRef(fullCategoryName);
}
}
+ ~Remark() = default;
// Remark argument that is a key-value pair that can be printed as machine
// parsable args.
@@ -168,6 +174,8 @@ class Remark {
StringRef getRemarkTypeString() const;
+ bool isPostponed() const { return postponed; }
+
protected:
/// Keeps the MLIR diagnostic kind, which is used to determine the
/// diagnostic kind in the LLVM remark streamer.
@@ -191,6 +199,9 @@ class Remark {
/// Args collected via the streaming interface.
SmallVector<Arg, 4> args;
+ /// Whether the remark is postponed (to be shown later).
+ bool postponed = false;
+
private:
/// Convert the MLIR diagnostic severity to LLVM diagnostic severity.
static llvm::DiagnosticSeverity
@@ -344,6 +355,8 @@ class MLIRRemarkStreamerBase {
class RemarkEngine {
private:
+ /// Postponed remarks. They are shown at the end of the pipeline.
+ llvm::SmallVector<Remark, 8> postponedRemarks;
/// Regex that filters missed optimization remarks: only matching one are
/// reported.
std::optional<llvm::Regex> missFilter;
@@ -392,6 +405,8 @@ class RemarkEngine {
InFlightRemark emitIfEnabled(Location loc, RemarkOpts opts,
bool (RemarkEngine::*isEnabled)(StringRef)
const);
+ /// Emit all postponed remarks.
+ void emitPostponedRemarks();
public:
/// Default constructor is deleted, use the other constructor.
@@ -411,7 +426,7 @@ class RemarkEngine {
std::string *errMsg);
/// Report a remark.
- void report(const Remark &&remark);
+ void report(const Remark &remark, bool ignorePostpone = false);
/// Report a successful remark, this will create an InFlightRemark
/// that can be used to build the remark using the << operator.
diff --git a/mlir/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp
index a55f61aff77bb..04799812e544c 100644
--- a/mlir/lib/IR/Remarks.cpp
+++ b/mlir/lib/IR/Remarks.cpp
@@ -225,7 +225,13 @@ InFlightRemark RemarkEngine::emitOptimizationRemarkAnalysis(Location loc,
// RemarkEngine
//===----------------------------------------------------------------------===//
-void RemarkEngine::report(const Remark &&remark) {
+void RemarkEngine::report(const Remark &remark, bool ignorePostpone) {
+ // Postponed remarks are shown at the end of pipeline, unless overridden.
+ if (remark.isPostponed() && !ignorePostpone) {
+ postponedRemarks.push_back(remark);
+ return;
+ }
+
// Stream the remark
if (remarkStreamer)
remarkStreamer->streamOptimizationRemark(remark);
@@ -235,7 +241,15 @@ void RemarkEngine::report(const Remark &&remark) {
emitRemark(remark.getLocation(), remark.getMsg());
}
+void RemarkEngine::emitPostponedRemarks() {
+ for (auto &remark : postponedRemarks)
+ report(remark, /*ignorePostpone=*/true);
+ postponedRemarks.clear();
+}
+
RemarkEngine::~RemarkEngine() {
+ emitPostponedRemarks();
+
if (remarkStreamer)
remarkStreamer->finalize();
}
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index 5bfca255c22ca..e842d4e1823d7 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -315,4 +315,91 @@ TEST(Remark, TestCustomOptimizationRemarkDiagnostic) {
EXPECT_NE(errOut.find(pass2Msg), std::string::npos); // printed
EXPECT_EQ(errOut.find(pass3Msg), std::string::npos); // filtered out
}
+
+TEST(Remark, TestCustomOptimizationRemarkPostponeDiagnostic) {
+ testing::internal::CaptureStderr();
+ const auto *pass1Msg = "My message";
+ const auto *pass2Msg = "My another message";
+ const auto *pass3Msg = "Do not show this message";
+
+ std::string categoryLoopunroll("LoopUnroll");
+ std::string categoryInline("Inliner");
+ std::string myPassname1("myPass1");
+ std::string myPassname2("myPass2");
+ std::string funcName("myFunc");
+
+ std::string seenMsg = "";
+
+ {
+ MLIRContext context;
+ Location loc = UnknownLoc::get(&context);
+
+ // Setup the remark engine
+ mlir::remark::RemarkCategories cats{/*passed=*/categoryLoopunroll,
+ /*missed=*/std::nullopt,
+ /*analysis=*/std::nullopt,
+ /*failed=*/categoryLoopunroll};
+
+ LogicalResult isEnabled = remark::enableOptimizationRemarks(
+ context, std::make_unique<MyCustomStreamer>(), cats, true);
+ ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
+
+ // Remark 1: pass, category LoopUnroll
+ remark::passed(loc, remark::RemarkOpts::name("")
+ .category(categoryLoopunroll)
+ .subCategory(myPassname2)
+ .postpone())
+ << pass1Msg;
+
+ // Postponed remark should not be printed yet.
+ testing::internal::CaptureStderr();
+ llvm::errs().flush();
+ std::string errOut1 = testing::internal::GetCapturedStderr();
+ // Ensure no remark has been printed yet.
+ EXPECT_TRUE(errOut1.empty())
+ << "Expected no stderr output before postponed remarks are flushed";
+
+ // Remark 2: failure, category LoopUnroll
+ remark::failed(loc, remark::RemarkOpts::name("")
+ .category(categoryLoopunroll)
+ .subCategory(myPassname2)
+ .postpone())
+ << remark::reason(pass2Msg);
+
+ // Postponed remark should not be printed yet.
+ testing::internal::CaptureStderr();
+ llvm::errs().flush();
+ std::string errOut2 = testing::internal::GetCapturedStderr();
+ // Ensure no remark has been printed yet.
+ EXPECT_TRUE(errOut2.empty())
+ << "Expected no stderr output before postponed remarks are flushed";
+
+ // Remark 3: pass, category Inline (should not be printed)
+ remark::passed(loc, remark::RemarkOpts::name("")
+ .category(categoryInline)
+ .subCategory(myPassname1))
+ << pass3Msg;
+
+ testing::internal::CaptureStderr();
+ llvm::errs().flush();
+ std::string errOut = testing::internal::GetCapturedStderr();
+ auto third = errOut.find("Custom remark:");
+ EXPECT_EQ(third, std::string::npos);
+ }
+ testing::internal::CaptureStderr();
+ llvm::errs().flush();
+ std::string errOut = ::testing::internal::GetCapturedStderr();
+
+ // Expect exactly two "Custom remark:" lines.
+ auto first = errOut.find("Custom remark:");
+ EXPECT_NE(first, std::string::npos);
+ auto second = errOut.find("Custom remark:", first + 1);
+ EXPECT_NE(second, std::string::npos);
+
+ // Containment checks for messages.
+ EXPECT_NE(errOut.find(pass1Msg), std::string::npos); // printed
+ EXPECT_NE(errOut.find(pass2Msg), std::string::npos); // printed
+ EXPECT_EQ(errOut.find(pass3Msg), std::string::npos); // filtered out
+}
+
} // namespace
>From 65ec3a9a38649a7ad5ea93e4994f443f815b70a2 Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Mon, 8 Sep 2025 15:25:19 +0200
Subject: [PATCH 2/6] fx
---
mlir/include/mlir/IR/Remarks.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 94fce3adb32cf..f49ccd8262eeb 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -78,7 +78,7 @@ struct RemarkOpts {
constexpr RemarkOpts function(StringRef v) const {
return {remarkName, categoryName, subCategoryName, v, postponed};
}
- /// Return a copy with the function name set.
+ /// Return a copy with the postponed flag set.
constexpr RemarkOpts postpone() const {
return {remarkName, categoryName, subCategoryName, functionName, true};
}
@@ -103,7 +103,6 @@ class Remark {
.toStringRef(fullCategoryName);
}
}
- ~Remark() = default;
// Remark argument that is a key-value pair that can be printed as machine
// parsable args.
>From a8c4141b6e06a89e2d14b4da344134c42fe228e7 Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Mon, 8 Sep 2025 16:36:37 +0200
Subject: [PATCH 3/6] fx
---
mlir/lib/IR/MLIRContext.cpp | 3 ++
mlir/unittests/IR/RemarkTest.cpp | 88 ++++++++++++++++----------------
2 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 1fa04ed8e738f..b9c8bf1de6787 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -278,6 +278,9 @@ class MLIRContextImpl {
}
}
~MLIRContextImpl() {
+ // finalize remark engine before destroying anything else.
+ remarkEngine.reset();
+
for (auto typeMapping : registeredTypes)
typeMapping.second->~AbstractType();
for (auto attrMapping : registeredAttributes)
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index e842d4e1823d7..74c941b733be8 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -323,13 +323,10 @@ TEST(Remark, TestCustomOptimizationRemarkPostponeDiagnostic) {
const auto *pass3Msg = "Do not show this message";
std::string categoryLoopunroll("LoopUnroll");
- std::string categoryInline("Inliner");
std::string myPassname1("myPass1");
std::string myPassname2("myPass2");
std::string funcName("myFunc");
- std::string seenMsg = "";
-
{
MLIRContext context;
Location loc = UnknownLoc::get(&context);
@@ -344,49 +341,53 @@ TEST(Remark, TestCustomOptimizationRemarkPostponeDiagnostic) {
context, std::make_unique<MyCustomStreamer>(), cats, true);
ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
- // Remark 1: pass, category LoopUnroll
- remark::passed(loc, remark::RemarkOpts::name("")
- .category(categoryLoopunroll)
- .subCategory(myPassname2)
- .postpone())
- << pass1Msg;
-
- // Postponed remark should not be printed yet.
- testing::internal::CaptureStderr();
- llvm::errs().flush();
- std::string errOut1 = testing::internal::GetCapturedStderr();
- // Ensure no remark has been printed yet.
- EXPECT_TRUE(errOut1.empty())
- << "Expected no stderr output before postponed remarks are flushed";
-
- // Remark 2: failure, category LoopUnroll
- remark::failed(loc, remark::RemarkOpts::name("")
- .category(categoryLoopunroll)
- .subCategory(myPassname2)
- .postpone())
- << remark::reason(pass2Msg);
-
// Postponed remark should not be printed yet.
- testing::internal::CaptureStderr();
- llvm::errs().flush();
- std::string errOut2 = testing::internal::GetCapturedStderr();
- // Ensure no remark has been printed yet.
- EXPECT_TRUE(errOut2.empty())
- << "Expected no stderr output before postponed remarks are flushed";
-
- // Remark 3: pass, category Inline (should not be printed)
- remark::passed(loc, remark::RemarkOpts::name("")
- .category(categoryInline)
- .subCategory(myPassname1))
- << pass3Msg;
+ // Remark 1: pass, category LoopUnroll
+ {
+ remark::passed(loc, remark::RemarkOpts::name("")
+ .category(categoryLoopunroll)
+ .subCategory(myPassname2)
+ .postpone())
+ << pass1Msg;
+ llvm::errs().flush();
+ std::string errOut1 = testing::internal::GetCapturedStderr();
+ // Ensure no remark has been printed yet.
+ EXPECT_TRUE(errOut1.empty())
+ << "Expected no stderr output before postponed remarks are flushed";
+ }
+ {
+ // Postponed remark should not be printed yet.
+ testing::internal::CaptureStderr();
+ // Remark 2: failure, category LoopUnroll
+ remark::failed(loc, remark::RemarkOpts::name("")
+ .category(categoryLoopunroll)
+ .subCategory(myPassname2)
+ .postpone())
+ << remark::reason(pass2Msg);
+
+ llvm::errs().flush();
+ std::string errOut2 = testing::internal::GetCapturedStderr();
+ // Ensure no remark has been printed yet.
+ EXPECT_TRUE(errOut2.empty())
+ << "Expected no stderr output before postponed remarks are flushed";
+ }
+ {
+ // Remark 3: pass, category Inline (should be printed)
+ testing::internal::CaptureStderr();
+ remark::passed(loc, remark::RemarkOpts::name("")
+ .category(categoryLoopunroll)
+ .subCategory(myPassname1))
+ << pass3Msg;
+
+ llvm::errs().flush();
+ std::string errOut = testing::internal::GetCapturedStderr();
+ auto third = errOut.find("Custom remark:");
+ EXPECT_NE(third, std::string::npos);
+ }
testing::internal::CaptureStderr();
- llvm::errs().flush();
- std::string errOut = testing::internal::GetCapturedStderr();
- auto third = errOut.find("Custom remark:");
- EXPECT_EQ(third, std::string::npos);
}
- testing::internal::CaptureStderr();
+
llvm::errs().flush();
std::string errOut = ::testing::internal::GetCapturedStderr();
@@ -395,11 +396,12 @@ TEST(Remark, TestCustomOptimizationRemarkPostponeDiagnostic) {
EXPECT_NE(first, std::string::npos);
auto second = errOut.find("Custom remark:", first + 1);
EXPECT_NE(second, std::string::npos);
+ auto third = errOut.find("Custom remark:", second + 1);
+ EXPECT_EQ(third, std::string::npos);
// Containment checks for messages.
EXPECT_NE(errOut.find(pass1Msg), std::string::npos); // printed
EXPECT_NE(errOut.find(pass2Msg), std::string::npos); // printed
- EXPECT_EQ(errOut.find(pass3Msg), std::string::npos); // filtered out
}
} // namespace
>From 805980bca25f4b26ca0d7254475410b4b509dc8d Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Tue, 16 Sep 2025 09:54:51 +0200
Subject: [PATCH 4/6] fx
---
mlir/include/mlir/IR/Remarks.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index f49ccd8262eeb..d6237fdbc2ca5 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/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/Remarks/Remark.h"
@@ -442,6 +443,12 @@ class RemarkEngine {
/// Report an analysis remark, this will create an InFlightRemark
/// that can be used to build the remark using the << operator.
InFlightRemark emitOptimizationRemarkAnalysis(Location loc, RemarkOpts opts);
+
+ /// Get the postponed remarks.
+ ArrayRef<Remark> getPostponedRemarks() const { return postponedRemarks; }
+
+ /// Clear the postponed remarks.
+ void clearPostponedRemarks() { postponedRemarks.clear(); }
};
template <typename Fn, typename... Args>
>From 3f66a95a247d62df82486e5c568a6e3711a67f9d Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Tue, 16 Sep 2025 10:28:29 +0200
Subject: [PATCH 5/6] fx
---
mlir/include/mlir/IR/Remarks.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index d6237fdbc2ca5..2b6e4a9110b9b 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -445,10 +445,10 @@ class RemarkEngine {
InFlightRemark emitOptimizationRemarkAnalysis(Location loc, RemarkOpts opts);
/// Get the postponed remarks.
- ArrayRef<Remark> getPostponedRemarks() const { return postponedRemarks; }
+ ArrayRef<Remark> getPostponedRemarks() const { return postponedRemarks; }
/// Clear the postponed remarks.
- void clearPostponedRemarks() { postponedRemarks.clear(); }
+ void clearPostponedRemarks() { postponedRemarks.clear(); }
};
template <typename Fn, typename... Args>
>From 40c182edc80df0a0cbf3e7fc1918fdc031e29ebd Mon Sep 17 00:00:00 2001
From: Guray Ozen <gozen at nvidia.com>
Date: Tue, 16 Sep 2025 10:49:18 +0200
Subject: [PATCH 6/6] fx
---
mlir/unittests/IR/RemarkTest.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index 74c941b733be8..ece7b9fb8624a 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -280,7 +280,7 @@ TEST(Remark, TestCustomOptimizationRemarkDiagnostic) {
Location loc = UnknownLoc::get(&context);
// Setup the remark engine
- mlir::remark::RemarkCategories cats{/*all=*/"",
+ mlir::remark::RemarkCategories cats{/*all=*/std::nullopt,
/*passed=*/categoryLoopunroll,
/*missed=*/std::nullopt,
/*analysis=*/std::nullopt,
@@ -332,7 +332,8 @@ TEST(Remark, TestCustomOptimizationRemarkPostponeDiagnostic) {
Location loc = UnknownLoc::get(&context);
// Setup the remark engine
- mlir::remark::RemarkCategories cats{/*passed=*/categoryLoopunroll,
+ mlir::remark::RemarkCategories cats{/*all=*/std::nullopt,
+ /*passed=*/categoryLoopunroll,
/*missed=*/std::nullopt,
/*analysis=*/std::nullopt,
/*failed=*/categoryLoopunroll};
More information about the Mlir-commits
mailing list