[Mlir-commits] [mlir] [MLIR] Add option to postpone remark emission (PR #157434)
Guray Ozen
llvmlistbot at llvm.org
Mon Sep 8 07:36:49 PDT 2025
https://github.com/grypp updated https://github.com/llvm/llvm-project/pull/157434
>From a741093a970995436db56156a15f58ed97c41872 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/3] [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 26d65472f2b1c..8879a5f5bb662 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 78c964427868f..3307be31bec6b 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 65e1e08b83838..ca506fcf0c0ed 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -312,4 +312,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 05b765e5d084cc0c9868aa0547bb4ee07d9702b4 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/3] 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 8879a5f5bb662..5eb691cd7c3ba 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 e066b9973956b2dfe5611033e0bcc223777ab1f2 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/3] 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 ca506fcf0c0ed..b3feafce54750 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -320,13 +320,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);
@@ -341,49 +338,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();
@@ -392,11 +393,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
More information about the Mlir-commits
mailing list