diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h index 20e84ec83cd01..c74070122b052 100644 --- a/mlir/include/mlir/IR/Remarks.h +++ b/mlir/include/mlir/IR/Remarks.h @@ -13,6 +13,8 @@ #ifndef MLIR_IR_REMARKS_H #define MLIR_IR_REMARKS_H +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/Hashing.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/Remarks/Remark.h" @@ -21,6 +23,7 @@ #include #include "mlir/IR/Diagnostics.h" +#include "mlir/IR/Location.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/Value.h" @@ -60,22 +63,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 postponed flag set. + constexpr RemarkOpts postpone() const { + return {remarkName, categoryName, subCategoryName, functionName, true}; } }; @@ -92,10 +100,10 @@ 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); + .toStringRef(combinedCategoryName); } } @@ -144,14 +152,14 @@ class Remark { llvm::StringRef getCategoryName() const { return categoryName; } - llvm::StringRef getFullCategoryName() const { + llvm::StringRef getCombinedCategoryName() const { if (categoryName.empty() && subCategoryName.empty()) return {}; if (subCategoryName.empty()) return categoryName; if (categoryName.empty()) return subCategoryName; - return fullCategoryName; + return combinedCategoryName; } StringRef getRemarkName() const { @@ -168,6 +176,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. @@ -183,7 +193,7 @@ class Remark { StringRef subCategoryName; /// Combined name for category and sub-category - SmallString<64> fullCategoryName; + SmallString<64> combinedCategoryName; /// Remark identifier StringRef remarkName; @@ -191,6 +201,9 @@ class Remark { /// Args collected via the streaming interface. SmallVector 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 +357,10 @@ class MLIRRemarkStreamerBase { class RemarkEngine { private: + /// Postponed remarks. They are deferred to the end of the pipeline, where the + /// user can intercept them for custom processing, otherwise they will be + /// reported on engine destruction. + llvm::DenseSet postponedRemarks; /// Regex that filters missed optimization remarks: only matching one are /// reported. std::optional missFilter; @@ -392,6 +409,12 @@ class RemarkEngine { InFlightRemark emitIfEnabled(Location loc, RemarkOpts opts, bool (RemarkEngine::*isEnabled)(StringRef) const); + /// Emit all postponed remarks. + void emitPostponedRemarks(); + + /// Report a remark. When `forcePrintPostponedRemarks` is true, the remark + /// will be printed even if it is postponed. + void reportImpl(const Remark &remark); public: /// Default constructor is deleted, use the other constructor. @@ -411,7 +434,7 @@ class RemarkEngine { std::string *errMsg); /// Report a remark. - void report(const Remark &&remark); + void report(const Remark &remark); /// Report a successful remark, this will create an InFlightRemark /// that can be used to build the remark using the << operator. @@ -428,6 +451,17 @@ 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. + const DenseSet &getPostponedRemarks() const { return postponedRemarks; } + + /// Clear the postponed remarks. + void clearPostponedRemarks() { postponedRemarks.clear(); } + + /// Drop a postponed remark. + bool dropPostponedRemark(Remark &remark) { + return postponedRemarks.erase(remark); + } }; template @@ -498,6 +532,21 @@ inline detail::InFlightRemark analysis(Location loc, RemarkOpts opts) { return withEngine(&detail::RemarkEngine::emitOptimizationRemarkAnalysis, loc, opts); } +//===----------------------------------------------------------------------===// +// Utils +//===----------------------------------------------------------------------===// + +/// Drop a postponed remark. +inline bool dropPostponedRemark(Location loc, RemarkKind remarkKind, + RemarkOpts opts) { + MLIRContext *ctx = loc->getContext(); + detail::Remark remark(remarkKind, DiagnosticSeverity::Remark, loc, opts); + + // Drop the remark from the postponed remarks. + if (detail::RemarkEngine *engine = ctx->getRemarkEngine()) + return engine->dropPostponedRemark(remark); + return false; +} //===----------------------------------------------------------------------===// // Setup @@ -517,4 +566,64 @@ LogicalResult enableOptimizationRemarks( } // namespace mlir::remark +//===----------------------------------------------------------------------===// +// DenseMapInfo specialization for Remark +//===----------------------------------------------------------------------===// + +namespace llvm { +template <> +struct DenseMapInfo { + static constexpr StringRef kEmptyKey = ""; + static constexpr StringRef kTombstoneKey = ""; + + /// Helper to provide a static dummy context for sentinel keys. + static mlir::MLIRContext *getStaticDummyContext() { + static mlir::MLIRContext dummyContext; + return &dummyContext; + } + + /// Create an empty remark + static inline mlir::remark::detail::Remark getEmptyKey() { + return mlir::remark::detail::Remark( + mlir::remark::RemarkKind::RemarkUnknown, mlir::DiagnosticSeverity::Note, + mlir::UnknownLoc::get(getStaticDummyContext()), + mlir::remark::RemarkOpts::name(kEmptyKey)); + } + + /// Create a dead remark + static inline mlir::remark::detail::Remark getTombstoneKey() { + return mlir::remark::detail::Remark( + mlir::remark::RemarkKind::RemarkUnknown, mlir::DiagnosticSeverity::Note, + mlir::UnknownLoc::get(getStaticDummyContext()), + mlir::remark::RemarkOpts::name(kTombstoneKey)); + } + + /// Compute the hash value of the remark + static unsigned getHashValue(const mlir::remark::detail::Remark &remark) { + return llvm::hash_combine( + remark.getLocation().getAsOpaquePointer(), + llvm::hash_value(remark.getRemarkName()), + llvm::hash_value(remark.getCombinedCategoryName()), + static_cast(remark.getRemarkType())); + } + + static bool isEqual(const mlir::remark::detail::Remark &lhs, + const mlir::remark::detail::Remark &rhs) { + // Check for empty/tombstone keys first + if (lhs.getRemarkName() == kEmptyKey || + lhs.getRemarkName() == kTombstoneKey || + rhs.getRemarkName() == kEmptyKey || + rhs.getRemarkName() == kTombstoneKey) { + return lhs.getRemarkName() == rhs.getRemarkName(); + } + + // For regular remarks, compare key identifying fields + return lhs.getLocation() == rhs.getLocation() && + lhs.getRemarkName() == rhs.getRemarkName() && + lhs.getCombinedCategoryName() == rhs.getCombinedCategoryName() && + lhs.getRemarkType() == rhs.getRemarkType(); + } +}; +} // namespace llvm + #endif // MLIR_IR_REMARKS_H 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/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp index a55f61aff77bb..9fc8193873ffd 100644 --- a/mlir/lib/IR/Remarks.cpp +++ b/mlir/lib/IR/Remarks.cpp @@ -70,7 +70,7 @@ static void printArgs(llvm::raw_ostream &os, llvm::ArrayRef args) { void Remark::print(llvm::raw_ostream &os, bool printLocation) const { // Header: [Type] pass:remarkName StringRef type = getRemarkTypeString(); - StringRef categoryName = getFullCategoryName(); + StringRef categoryName = getCombinedCategoryName(); StringRef name = remarkName; os << '[' << type << "] "; @@ -140,7 +140,7 @@ llvm::remarks::Remark Remark::generateRemark() const { r.RemarkType = getRemarkType(); r.RemarkName = getRemarkName(); // MLIR does not use passes; instead, it has categories and sub-categories. - r.PassName = getFullCategoryName(); + r.PassName = getCombinedCategoryName(); r.FunctionName = getFunction(); r.Loc = locLambda(); for (const Remark::Arg &arg : getArgs()) { @@ -157,7 +157,7 @@ llvm::remarks::Remark Remark::generateRemark() const { InFlightRemark::~InFlightRemark() { if (remark && owner) - owner->report(std::move(*remark)); + owner->report(*remark); owner = nullptr; } @@ -225,7 +225,7 @@ InFlightRemark RemarkEngine::emitOptimizationRemarkAnalysis(Location loc, // RemarkEngine //===----------------------------------------------------------------------===// -void RemarkEngine::report(const Remark &&remark) { +void RemarkEngine::reportImpl(const Remark &remark) { // Stream the remark if (remarkStreamer) remarkStreamer->streamOptimizationRemark(remark); @@ -235,7 +235,25 @@ void RemarkEngine::report(const Remark &&remark) { emitRemark(remark.getLocation(), remark.getMsg()); } +void RemarkEngine::report(const Remark &remark) { + // Postponed remarks are deferred to the end of pipeline. + if (remark.isPostponed()) { + postponedRemarks.insert(remark); + return; + } + + reportImpl(remark); +} + +void RemarkEngine::emitPostponedRemarks() { + for (auto &remark : postponedRemarks) + reportImpl(remark); + 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..0d8bd2e2937a6 100644 --- a/mlir/unittests/IR/RemarkTest.cpp +++ b/mlir/unittests/IR/RemarkTest.cpp @@ -10,9 +10,7 @@ #include "mlir/IR/MLIRContext.h" #include "mlir/IR/Remarks.h" #include "mlir/Remark/RemarkStreamer.h" -#include "mlir/Support/TypeID.h" #include "llvm/ADT/StringRef.h" -#include "llvm/IR/LLVMRemarkStreamer.h" #include "llvm/Remarks/RemarkFormat.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/LogicalResult.h" @@ -280,7 +278,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, @@ -315,4 +313,202 @@ 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 myPassname1("myPass1"); + std::string myPassname2("myPass2"); + std::string funcName("myFunc"); + + { + MLIRContext context; + Location loc = UnknownLoc::get(&context); + + // Setup the remark engine + mlir::remark::RemarkCategories cats{/*all=*/std::nullopt, + /*passed=*/categoryLoopunroll, + /*missed=*/std::nullopt, + /*analysis=*/std::nullopt, + /*failed=*/categoryLoopunroll}; + + LogicalResult isEnabled = remark::enableOptimizationRemarks( + context, std::make_unique(), cats, true); + ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine"; + + // Postponed remark should not be printed yet. + // 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(); + + // 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); + 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 +} + +TEST(Remark, TestRemarkDrop) { + testing::internal::CaptureStderr(); + const auto *pass1Msg = "I am failed"; + const auto *pass2Msg = "I succeeded"; + std::string categoryLoopunroll("LoopUnroll"); + { + MLIRContext context; + Location loc = FileLineColLoc::get(&context, "test.cpp", 1, 5); + Location locOther = FileLineColLoc::get(&context, "test.cpp", 55, 5); + + // Setup the remark engine + mlir::remark::RemarkCategories cats{/*all=*/categoryLoopunroll, + /*passed=*/categoryLoopunroll, + /*missed=*/categoryLoopunroll, + /*analysis=*/categoryLoopunroll, + /*failed=*/categoryLoopunroll}; + + LogicalResult isEnabled = remark::enableOptimizationRemarks( + context, std::make_unique(), cats, true); + ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine"; + + // [Pass-early]: Failed remark is emitted as postponed but not shown + { + remark::failed(loc, remark::RemarkOpts::name("unroller") + .category(categoryLoopunroll) + .postpone()) + << pass1Msg; + + // Ensure no remark has been printed yet. + llvm::errs().flush(); + std::string errOut1 = testing::internal::GetCapturedStderr(); + EXPECT_TRUE(errOut1.empty()) + << "Expected no stderr output before postponed remarks are flushed"; + } + testing::internal::CaptureStderr(); + // [Pass-late]: Drop the failed remark, and emit a passed remark + { + // Step 1. Drop the remark from early-pass + bool dropped = remark::dropPostponedRemark( + loc, remark::RemarkKind::RemarkFailure, + remark::RemarkOpts::name("unroller").category(categoryLoopunroll)); + + EXPECT_TRUE(dropped) << "Expected the remark to be dropped"; + + // Step 2. Print the pass remark + remark::passed( + loc, + remark::RemarkOpts::name("unroller").category(categoryLoopunroll)) + << pass2Msg; + } + } + // done with the compilation + llvm::errs().flush(); + std::string errOut2 = testing::internal::GetCapturedStderr(); + + EXPECT_EQ(errOut2.find(pass1Msg), std::string::npos); + EXPECT_NE(errOut2.find(pass2Msg), std::string::npos); +} + +TEST(Remark, TestRemarkCannotDrop) { + testing::internal::CaptureStderr(); + const auto *pass1Msg = "I am failed"; + const auto *pass2Msg = "I succeeded"; + std::string categoryLoopunroll("LoopUnroll"); + { + MLIRContext context; + Location loc = FileLineColLoc::get(&context, "test.cpp", 1, 5); + Location locOther = FileLineColLoc::get(&context, "test.cpp", 55, 5); + // Setup the remark engine + mlir::remark::RemarkCategories cats{/*all=*/categoryLoopunroll, + /*passed=*/categoryLoopunroll, + /*missed=*/categoryLoopunroll, + /*analysis=*/categoryLoopunroll, + /*failed=*/categoryLoopunroll}; + + LogicalResult isEnabled = remark::enableOptimizationRemarks( + context, std::make_unique(), cats, true); + ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine"; + // [Pass-early]: Failed remark is emitted as postponed but not shown + { + remark::failed(locOther, remark::RemarkOpts::name("unroller") + .category(categoryLoopunroll) + .postpone()) + << pass1Msg; + } + // [Pass-late]: Drop the failed remark, and emit a passed remark + { + // Step 1. Drop the remark from early-pass + bool dropped = remark::dropPostponedRemark( + loc, remark::RemarkKind::RemarkFailure, + remark::RemarkOpts::name("unroller").category(categoryLoopunroll)); + + EXPECT_FALSE(dropped) << "Expected the remark should not be dropped it " + "has different location"; + + // Step 2. Print the pass remark + remark::passed( + loc, + remark::RemarkOpts::name("unroller").category(categoryLoopunroll)) + << pass2Msg; + } + } + // done with the compilation + llvm::errs().flush(); + std::string errOut3 = testing::internal::GetCapturedStderr(); + EXPECT_NE(errOut3.find(pass1Msg), std::string::npos); // printed + EXPECT_NE(errOut3.find(pass2Msg), std::string::npos); // printed +} } // namespace