Skip to content

Conversation

grypp
Copy link
Member

@grypp grypp commented Sep 22, 2025

PR introduces RemarkPolicy to show remarks and drop.

A very typical compiler diagnostic scenario is shown below. In this example, Remark-1 is incorrect, because pass-late actually unrolled the loop.

    [pass-early] : emit Remark-1: failed unrolling, postponed
    ... other passes ...
    [pass-late]  : emit Remark-2: succeeded unrolling // same location/op

PR introduces RemarkPolicy to the RemarkEngine and adds an option --remark-policy=all|final

  • all : shows Remark-1 and Remark-2.
  • final : RemarkEngine postpones all remarks by default, and automatically drops earlier ones, keeping only the last remark per location + remark name + category.

@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Guray Ozen (grypp)

Changes

PR introduces RemarkPolicy to show remarks and drop.

A very typical compiler diagnostic scenario is shown below. In this example, Remark-1 is incorrect, because pass-late actually unrolled the loop.

    [pass-early] : emit Remark-1: failed unrolling, postponed
    ... other passes ...
    [pass-late]  : emit Remark-2: succeeded unrolling // same location/op

PR introduces RemarkPolicy to the RemarkEngine and adds an option --remark-policy=all|final

  • all : shows Remark-1 and Remark-2.
  • final : RemarkEngine postpones all remarks by default, and automatically drops earlier ones, keeping only the last remark per location/op.

Patch is 20.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160062.diff

8 Files Affected:

  • (modified) mlir/include/mlir/IR/Remarks.h (+84-4)
  • (modified) mlir/include/mlir/Remark/RemarkStreamer.h (+1-1)
  • (modified) mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h (+9)
  • (modified) mlir/lib/IR/MLIRContext.cpp (+2)
  • (modified) mlir/lib/IR/Remarks.cpp (+33-14)
  • (modified) mlir/lib/Remark/RemarkStreamer.cpp (+2-2)
  • (modified) mlir/lib/Tools/mlir-opt/MlirOptMain.cpp (+21-3)
  • (modified) mlir/unittests/IR/RemarkTest.cpp (+76-6)
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 20e84ec83cd01..6e602ea181ccc 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -32,6 +32,20 @@ struct RemarkCategories {
   std::optional<std::string> all, passed, missed, analysis, failed;
 };
 
+/// Define the policy to use for the remark engine.
+enum RemarkPolicy {
+  RemarkPolicyAll = 0,
+  RemarkPolicyFinal = 1,
+};
+
+/// Define the operations to perform on the remark engine.
+/// By default none are, the provided regex matches against the category names
+/// for each kind of remark.
+struct RemarkEngineOpts {
+  RemarkCategories categories;
+  RemarkPolicy policy;
+};
+
 /// Categories describe the outcome of an transformation, not the mechanics of
 /// emitting/serializing remarks.
 enum class RemarkKind {
@@ -144,7 +158,7 @@ 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())
@@ -344,6 +358,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<Remark> postponedRemarks;
   /// Regex that filters missed optimization remarks: only matching one are
   /// reported.
   std::optional<llvm::Regex> missFilter;
@@ -357,6 +375,8 @@ class RemarkEngine {
   std::unique_ptr<MLIRRemarkStreamerBase> remarkStreamer;
   /// When is enabled, engine also prints remarks as mlir::emitRemarks.
   bool printAsEmitRemarks = false;
+  /// The policy to use for the remark engine.
+  RemarkPolicy remarkPolicy = RemarkPolicy::RemarkPolicyAll;
 
   /// Return true if missed optimization remarks are enabled, override
   /// to provide different implementation.
@@ -392,6 +412,11 @@ class RemarkEngine {
   InFlightRemark emitIfEnabled(Location loc, RemarkOpts opts,
                                bool (RemarkEngine::*isEnabled)(StringRef)
                                    const);
+  /// Emit all postponed remarks.
+  void emitPostponedRemarks();
+
+  /// Report a remark.
+  void reportImpl(const Remark &remark);
 
 public:
   /// Default constructor is deleted, use the other constructor.
@@ -400,7 +425,7 @@ class RemarkEngine {
   /// Constructs Remark engine with optional category names. If a category
   /// name is not provided, it is not enabled. The category names are used to
   /// filter the remarks that are emitted.
-  RemarkEngine(bool printAsEmitRemarks, const RemarkCategories &cats);
+  RemarkEngine(bool printAsEmitRemarks, const RemarkEngineOpts &opts);
 
   /// Destructor that will close the output file and reset the
   /// main remark streamer.
@@ -410,7 +435,9 @@ class RemarkEngine {
   LogicalResult initialize(std::unique_ptr<MLIRRemarkStreamerBase> streamer,
                            std::string *errMsg);
 
-  /// Report a remark.
+  /// Report a remark. The remark is deferred to the end of the pipeline, where
+  /// the user can intercept them for custom processing, otherwise they will be
+  /// reported on engine destruction.
   void report(const Remark &&remark);
 
   /// Report a successful remark, this will create an InFlightRemark
@@ -513,8 +540,61 @@ inline detail::InFlightRemark analysis(Location loc, RemarkOpts opts) {
 LogicalResult enableOptimizationRemarks(
     MLIRContext &ctx,
     std::unique_ptr<remark::detail::MLIRRemarkStreamerBase> streamer,
-    const remark::RemarkCategories &cats, bool printAsEmitRemarks = false);
+    const remark::RemarkEngineOpts &opts, bool printAsEmitRemarks = false);
 
 } // namespace mlir::remark
 
+// DenseMapInfo specialization for Remark
+namespace llvm {
+template <>
+struct DenseMapInfo<mlir::remark::detail::Remark> {
+  static constexpr StringRef kEmptyKey = "<EMPTY_KEY>";
+  static constexpr StringRef kTombstoneKey = "<TOMBSTONE_KEY>";
+
+  /// 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.getCategoryName()));
+  }
+
+  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.getCategoryName() == rhs.getCategoryName();
+  }
+};
+} // namespace llvm
 #endif // MLIR_IR_REMARKS_H
diff --git a/mlir/include/mlir/Remark/RemarkStreamer.h b/mlir/include/mlir/Remark/RemarkStreamer.h
index 8bfd176d9bade..f3afdf37517cc 100644
--- a/mlir/include/mlir/Remark/RemarkStreamer.h
+++ b/mlir/include/mlir/Remark/RemarkStreamer.h
@@ -44,6 +44,6 @@ namespace mlir::remark {
 /// mlir::emitRemarks.
 LogicalResult enableOptimizationRemarksWithLLVMStreamer(
     MLIRContext &ctx, StringRef filePath, llvm::remarks::Format fmt,
-    const RemarkCategories &cat, bool printAsEmitRemarks = false);
+    const RemarkEngineOpts &opts, bool printAsEmitRemarks = false);
 
 } // namespace mlir::remark
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 0fbe15fa2e0db..b7394387b0f9a 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -44,6 +44,11 @@ enum class RemarkFormat {
   REMARK_FORMAT_BITSTREAM,
 };
 
+enum class RemarkPolicy {
+  REMARK_POLICY_ALL,
+  REMARK_POLICY_FINAL,
+};
+
 /// Configuration options for the mlir-opt tool.
 /// This is intended to help building tools like mlir-opt by collecting the
 /// supported options.
@@ -242,6 +247,8 @@ class MlirOptMainConfig {
 
   /// Set the reproducer output filename
   RemarkFormat getRemarkFormat() const { return remarkFormatFlag; }
+  /// Set the remark policy to use.
+  RemarkPolicy getRemarkPolicy() const { return remarkPolicyFlag; }
   /// Set the remark format to use.
   std::string getRemarksAllFilter() const { return remarksAllFilterFlag; }
   /// Set the remark output file.
@@ -265,6 +272,8 @@ class MlirOptMainConfig {
 
   /// Remark format
   RemarkFormat remarkFormatFlag = RemarkFormat::REMARK_FORMAT_STDOUT;
+  /// Remark policy
+  RemarkPolicy remarkPolicyFlag = RemarkPolicy::REMARK_POLICY_ALL;
   /// Remark file to output to
   std::string remarksOutputFileFlag = "";
   /// Remark filters
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 1fa04ed8e738f..358f0fc39b19d 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -278,6 +278,8 @@ 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..6e725a1718a01 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<Remark::Arg> 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()) {
@@ -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 (remarkPolicy == RemarkPolicy::RemarkPolicyFinal) {
+    bool erased = postponedRemarks.erase(remark);
+    postponedRemarks.insert(remark);
+    return;
+  }
+
+  reportImpl(remark);
+}
+
+void RemarkEngine::emitPostponedRemarks() {
+  for (auto &remark : postponedRemarks)
+    reportImpl(remark);
+  postponedRemarks.clear();
+}
+
 RemarkEngine::~RemarkEngine() {
+  emitPostponedRemarks();
   if (remarkStreamer)
     remarkStreamer->finalize();
 }
@@ -288,24 +306,25 @@ buildFilter(const mlir::remark::RemarkCategories &cats,
 }
 
 RemarkEngine::RemarkEngine(bool printAsEmitRemarks,
-                           const RemarkCategories &cats)
+                           const RemarkEngineOpts &opts)
     : printAsEmitRemarks(printAsEmitRemarks) {
-  if (cats.passed)
-    passedFilter = buildFilter(cats, cats.passed);
-  if (cats.missed)
-    missFilter = buildFilter(cats, cats.missed);
-  if (cats.analysis)
-    analysisFilter = buildFilter(cats, cats.analysis);
-  if (cats.failed)
-    failedFilter = buildFilter(cats, cats.failed);
+  if (opts.categories.passed)
+    passedFilter = buildFilter(opts.categories, opts.categories.passed);
+  if (opts.categories.missed)
+    missFilter = buildFilter(opts.categories, opts.categories.missed);
+  if (opts.categories.analysis)
+    analysisFilter = buildFilter(opts.categories, opts.categories.analysis);
+  if (opts.categories.failed)
+    failedFilter = buildFilter(opts.categories, opts.categories.failed);
+  remarkPolicy = opts.policy;
 }
 
 llvm::LogicalResult mlir::remark::enableOptimizationRemarks(
     MLIRContext &ctx,
     std::unique_ptr<remark::detail::MLIRRemarkStreamerBase> streamer,
-    const remark::RemarkCategories &cats, bool printAsEmitRemarks) {
+    const remark::RemarkEngineOpts &opts, bool printAsEmitRemarks) {
   auto engine =
-      std::make_unique<remark::detail::RemarkEngine>(printAsEmitRemarks, cats);
+      std::make_unique<remark::detail::RemarkEngine>(printAsEmitRemarks, opts);
 
   std::string errMsg;
   if (failed(engine->initialize(std::move(streamer), &errMsg))) {
diff --git a/mlir/lib/Remark/RemarkStreamer.cpp b/mlir/lib/Remark/RemarkStreamer.cpp
index 8e3544ff2c34c..8039af41da86a 100644
--- a/mlir/lib/Remark/RemarkStreamer.cpp
+++ b/mlir/lib/Remark/RemarkStreamer.cpp
@@ -55,14 +55,14 @@ LLVMRemarkStreamer::~LLVMRemarkStreamer() {
 namespace mlir::remark {
 LogicalResult enableOptimizationRemarksWithLLVMStreamer(
     MLIRContext &ctx, StringRef path, llvm::remarks::Format fmt,
-    const RemarkCategories &cat, bool printAsEmitRemarks) {
+    const RemarkEngineOpts &opts, bool printAsEmitRemarks) {
 
   FailureOr<std::unique_ptr<detail::MLIRRemarkStreamerBase>> sOr =
       detail::LLVMRemarkStreamer::createToFile(path, fmt);
   if (failed(sOr))
     return failure();
 
-  return remark::enableOptimizationRemarks(ctx, std::move(*sOr), cat,
+  return remark::enableOptimizationRemarks(ctx, std::move(*sOr), opts,
                                            printAsEmitRemarks);
 }
 
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 30fd384f3977c..8ccf41e765997 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -226,6 +226,18 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
                                     "bitstream", "Print bitstream file")),
         llvm::cl::cat(remarkCategory)};
 
+    static llvm::cl::opt<RemarkPolicy, /*ExternalStorage=*/true> remarkPolicy{
+        "remark-policy",
+        llvm::cl::desc("Specify the policy for remark output."),
+        cl::location(remarkPolicyFlag),
+        llvm::cl::value_desc("format"),
+        llvm::cl::init(RemarkPolicy::REMARK_POLICY_ALL),
+        llvm::cl::values(clEnumValN(RemarkPolicy::REMARK_POLICY_ALL, "all",
+                                    "Print all remarks"),
+                         clEnumValN(RemarkPolicy::REMARK_POLICY_FINAL, "final",
+                                    "Print final remarks")),
+        llvm::cl::cat(remarkCategory)};
+
     static cl::opt<std::string, /*ExternalStorage=*/true> remarksAll(
         "remarks-filter",
         cl::desc("Show all remarks: passed, missed, failed, analysis"),
@@ -518,17 +530,23 @@ performActions(raw_ostream &os,
 
   context->enableMultithreading(wasThreadingEnabled);
 
+  // Set the remark categories and policy.
   remark::RemarkCategories cats{
       config.getRemarksAllFilter(), config.getRemarksPassedFilter(),
       config.getRemarksMissedFilter(), config.getRemarksAnalyseFilter(),
       config.getRemarksFailedFilter()};
+  remark::RemarkPolicy policy =
+      config.getRemarkPolicy() == RemarkPolicy::REMARK_POLICY_FINAL
+          ? policy = remark::RemarkPolicy::RemarkPolicyFinal
+          : remark::RemarkPolicy::RemarkPolicyAll;
+  remark::RemarkEngineOpts opts{cats, policy};
 
   mlir::MLIRContext &ctx = *context;
 
   switch (config.getRemarkFormat()) {
   case RemarkFormat::REMARK_FORMAT_STDOUT:
     if (failed(mlir::remark::enableOptimizationRemarks(
-            ctx, nullptr, cats, true /*printAsEmitRemarks*/)))
+            ctx, nullptr, opts, true /*printAsEmitRemarks*/)))
       return failure();
     break;
 
@@ -537,7 +555,7 @@ performActions(raw_ostream &os,
                            ? "mlir-remarks.yaml"
                            : config.getRemarksOutputFile();
     if (failed(mlir::remark::enableOptimizationRemarksWithLLVMStreamer(
-            ctx, file, llvm::remarks::Format::YAML, cats)))
+            ctx, file, llvm::remarks::Format::YAML, opts)))
       return failure();
     break;
   }
@@ -547,7 +565,7 @@ performActions(raw_ostream &os,
                            ? "mlir-remarks.bitstream"
                            : config.getRemarksOutputFile();
     if (failed(mlir::remark::enableOptimizationRemarksWithLLVMStreamer(
-            ctx, file, llvm::remarks::Format::Bitstream, cats)))
+            ctx, file, llvm::remarks::Format::Bitstream, opts)))
       return failure();
     break;
   }
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index 5bfca255c22ca..9bb07321241ad 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -53,10 +53,11 @@ TEST(Remark, TestOutputOptimizationRemark) {
                                         /*missed=*/categoryUnroll,
                                         /*analysis=*/categoryRegister,
                                         /*failed=*/categoryInliner};
-
+    mlir::remark::RemarkEngineOpts opts{
+        cats, mlir::remark::RemarkPolicy::RemarkPolicyAll};
     LogicalResult isEnabled =
         mlir::remark::enableOptimizationRemarksWithLLVMStreamer(
-            context, yamlFile, llvm::remarks::Format::YAML, cats);
+            context, yamlFile, llvm::remarks::Format::YAML, opts);
     ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
 
     // PASS: something succeeded
@@ -203,9 +204,10 @@ TEST(Remark, TestOutputOptimizationRemarkDiagnostic) {
                                         /*missed=*/categoryUnroll,
                                         /*analysis=*/categoryRegister,
                                         /*failed=*/categoryUnroll};
-
+    mlir::remark::RemarkEngineOpts opts{
+        cats, mlir::remark::RemarkPolicy::RemarkPolicyAll};
     LogicalResult isEnabled =
-        remark::enableOptimizationRemarks(context, nullptr, cats, true);
+        remark::enableOptimizationRemarks(context, nullptr, opts, true);
 
     ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
 
@@ -285,9 +287,10 @@ TEST(Remark, TestCustomOptimizationRemarkDiagnostic) {
                                         /*missed=*/std::nullopt,
                                         /*analysis=*/std::nullopt,
                                         /*failed=*/categoryLoopunroll};
-
+    mlir::remark::RemarkEngineOpts opts{
+        cats, mlir::remark::RemarkPolicy::RemarkPolicyAll};
     LogicalResult isEnabled = remark::enableOptimizationRemarks(
-        context, std::make_unique<MyCustomStreamer>(), cats, true);
+        context, std::make_unique<MyCustomStreamer>(), opts, true);
     ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
 
     // Remark 1: pass, category LoopUnroll
@@ -315,4 +318,71 @@ TEST(Remark, TestCustomOptimizationRemarkDiagnostic) {
   EXPECT_NE(errOut.find(pass2Msg), std::string::npos); // printed
   EXPECT_EQ(errOut.find(pass3Msg), std::string::npos); // filtered out
 }
+
+TEST(Remark, TestRemarkFinal) {
+  testing::internal::CaptureStderr();
+  const auto *pass1Msg = "I failed";
+  const auto *pass2Msg = "I failed too";
+  const auto *pass3Msg = "I succeeded";
+  const auto *pass4Msg = "I succeeded too";
+
+  std::string categoryLoopunroll("LoopUnroll");
+  std::string myPassname1("myPass1");
+  std::string myPassname2("myPass2");
+  std::string myPassname3("myPass3");
+  std::string funcName("myFunc");
+
+  std::string seenMsg = "";
+
+  {
+    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=*/"",
+                                        /*passed=*/categoryLoopunroll,
+                                        /*missed=*/categoryLoopunroll,
+                                        /*analysis=*/categoryLoopunroll,
+                                        /*failed=*/categoryLoopunroll};
+    mlir::remark::RemarkEngineOpts opts{
+        cats, mlir::remark::RemarkPolicy::RemarkPolicyFinal};
+    LogicalResult isEnabled = remark::enableOptimizationRemarks(
+        context, std::make_unique<MyCustomStreamer>(), opts, true);
+    ASSERT_TRUE(succeeded(isEnabled)) << "Failed to enable remark engine";
+
+    // Remark 1: failure
+    remark::failed(loc, remark::RemarkOpts::name("Unroller")
+                            .category(categoryLoopunroll)
+                            .subCategory(myPassname1))
+        << pass1Msg;
+
+    // Remark 2: failure
+    remark::missed(loc, remark::RemarkOpts::name("Unroller")
+                            .category(categoryLoopunroll)
+                            .subCategory(myPassname2))
+  ...
[truncated]

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Introduces RemarkPolicy to enhance remark handling by providing control over when remarks are emitted. This allows filtering remarks to show only the final/latest remark per location when using the "final" policy, preventing misleading intermediate remarks that may later be superseded.

  • Adds RemarkPolicy enum with "all" and "final" options to control remark emission timing
  • Refactors RemarkEngine to accept RemarkEngineOpts containing both categories and policy
  • Implements postponed remark mechanism that defers and deduplicates remarks by location/name/category

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
mlir/include/mlir/IR/Remarks.h Defines RemarkPolicy enum, RemarkEngineOpts struct, and DenseMapInfo specialization for Remark deduplication
mlir/lib/IR/Remarks.cpp Implements postponed remark logic and updates RemarkEngine constructor to use RemarkEngineOpts
mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h Adds RemarkPolicy enum and configuration support for remark policy selection
mlir/lib/Tools/mlir-opt/MlirOptMain.cpp Integrates remark policy CLI option and updates function calls to use RemarkEngineOpts
mlir/include/mlir/Remark/RemarkStreamer.h Updates function signature to accept RemarkEngineOpts instead of RemarkCategories
mlir/lib/Remark/RemarkStreamer.cpp Updates function implementation to use RemarkEngineOpts parameter
mlir/lib/IR/MLIRContext.cpp Ensures remark engine finalization before context destruction
mlir/unittests/IR/RemarkTest.cpp Updates existing tests and adds new test for final remark policy behavior

@grypp grypp force-pushed the remark-postpone-flag branch from 4c395d1 to 61d40b5 Compare September 22, 2025 10:32
grypp and others added 8 commits September 23, 2025 09:09
PR introduces RemarkPolicy to show remarks and drop.

A very typical compiler diagnostic scenario is shown below. In this example, `Remark-1` is incorrect, because `pass-late` actually unrolled the loop.
```
    [pass-early] : emit Remark-1: failed unrolling, postponed
    ... other passes ...
    [pass-late]  : emit Remark-2: succeeded unrolling // same location/op
```

PR introduces `RemarkPolicy` to the RemarkEngine and adds an option `--remark-policy=all|final`
- `all` : shows Remark-1 and Remark-2.
- `final` : RemarkEngine postpones all remarks by default, and automatically drops earlier ones, keeping only the last remark per location/op.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@grypp grypp force-pushed the remark-postpone-flag branch from 575c952 to b413a75 Compare September 23, 2025 07:09
@joker-eph
Copy link
Collaborator

Is this to be closed now that #160526 is merged?

@grypp grypp closed this Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants