Skip to content

[MLIR] Migrate pattern application / dialect conversion to the LDBG logging format #150991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 1, 2025

Conversation

joker-eph
Copy link
Collaborator

This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the raw_ldbg_ostream to consistently prefix each new line.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir llvm:support labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-llvm-support

Author: Mehdi Amini (joker-eph)

Changes

This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the raw_ldbg_ostream to consistently prefix each new line.


Full diff: https://github.com/llvm/llvm-project/pull/150991.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Support/DebugLog.h (+9-4)
  • (modified) mlir/lib/Rewrite/PatternApplicator.cpp (+6-9)
  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+8-1)
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 8fca2d5e2816b..1b4fe9d13030f 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -85,7 +85,8 @@ namespace impl {
 class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   std::string Prefix;
   raw_ostream &Os;
-  bool HasPendingNewline = true;
+  bool HasPendingNewline;
+  bool ShouldPrintNewline;
 
   /// Split the line on newlines and insert the prefix before each newline.
   /// Forward everything to the underlying stream.
@@ -117,13 +118,17 @@ class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   }
 
 public:
-  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os)
-      : Prefix(std::move(Prefix)), Os(Os) {
+  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os,
+                            bool HasPendingNewline = true,
+                            bool ShouldPrintNewline = true)
+      : Prefix(std::move(Prefix)), Os(Os), HasPendingNewline(HasPendingNewline),
+        ShouldPrintNewline(ShouldPrintNewline) {
     SetUnbuffered();
   }
   ~raw_ldbg_ostream() final {
     flushEol();
-    Os << '\n';
+    if (ShouldPrintNewline)
+      Os << '\n';
   }
   void flushEol() {
     if (HasPendingNewline) {
diff --git a/mlir/lib/Rewrite/PatternApplicator.cpp b/mlir/lib/Rewrite/PatternApplicator.cpp
index b2b372b7b1249..e13bcff2767ec 100644
--- a/mlir/lib/Rewrite/PatternApplicator.cpp
+++ b/mlir/lib/Rewrite/PatternApplicator.cpp
@@ -13,7 +13,7 @@
 
 #include "mlir/Rewrite/PatternApplicator.h"
 #include "ByteCode.h"
-#include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 
 #ifndef NDEBUG
 #include "llvm/ADT/ScopeExit.h"
@@ -51,9 +51,7 @@ static Operation *getDumpRootOp(Operation *op) {
   return op;
 }
 static void logSucessfulPatternApplication(Operation *op) {
-  llvm::dbgs() << "// *** IR Dump After Pattern Application ***\n";
-  op->dump();
-  llvm::dbgs() << "\n\n";
+  LDBG(2) << "// *** IR Dump After Pattern Application ***\n" << *op << "\n";
 }
 #endif
 
@@ -208,8 +206,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
             result =
                 bytecode->rewrite(rewriter, *pdlMatch, *mutableByteCodeState);
           } else {
-            LLVM_DEBUG(llvm::dbgs() << "Trying to match \""
-                                    << bestPattern->getDebugName() << "\"\n");
+            LDBG() << "Trying to match \"" << bestPattern->getDebugName()
+                   << "\"";
             const auto *pattern =
                 static_cast<const RewritePattern *>(bestPattern);
 
@@ -223,9 +221,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
                 [&] { rewriter.setListener(oldListener); });
 #endif
             result = pattern->matchAndRewrite(op, rewriter);
-            LLVM_DEBUG(llvm::dbgs()
-                       << "\"" << bestPattern->getDebugName() << "\" result "
-                       << succeeded(result) << "\n");
+            LDBG() << " -> matchAndRewrite "
+                   << (succeeded(result) ? "successful" : "failed");
           }
 
           // Process the result of the pattern application.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 08803e082b057..abd75558e290f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -1129,8 +1130,14 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
   /// verification.
   SmallPtrSet<Operation *, 1> pendingRootUpdates;
 
+  /// A raw output stream used to prefix the debug log.
+  llvm::impl::raw_ldbg_ostream os{(Twine("[") + DEBUG_TYPE + "] ").str(),
+                                  llvm::dbgs(), /*HasPendingNewline=*/false,
+                                  /*ShouldPrintNewline=*/false};
+
   /// A logger used to emit diagnostics during the conversion process.
-  llvm::ScopedPrinter logger{llvm::dbgs()};
+  llvm::ScopedPrinter logger{os};
+  std::string logPrefix;
 #endif
 };
 } // namespace detail

@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

Changes

This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the raw_ldbg_ostream to consistently prefix each new line.


Full diff: https://github.com/llvm/llvm-project/pull/150991.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Support/DebugLog.h (+9-4)
  • (modified) mlir/lib/Rewrite/PatternApplicator.cpp (+6-9)
  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+8-1)
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 8fca2d5e2816b..1b4fe9d13030f 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -85,7 +85,8 @@ namespace impl {
 class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   std::string Prefix;
   raw_ostream &Os;
-  bool HasPendingNewline = true;
+  bool HasPendingNewline;
+  bool ShouldPrintNewline;
 
   /// Split the line on newlines and insert the prefix before each newline.
   /// Forward everything to the underlying stream.
@@ -117,13 +118,17 @@ class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   }
 
 public:
-  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os)
-      : Prefix(std::move(Prefix)), Os(Os) {
+  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os,
+                            bool HasPendingNewline = true,
+                            bool ShouldPrintNewline = true)
+      : Prefix(std::move(Prefix)), Os(Os), HasPendingNewline(HasPendingNewline),
+        ShouldPrintNewline(ShouldPrintNewline) {
     SetUnbuffered();
   }
   ~raw_ldbg_ostream() final {
     flushEol();
-    Os << '\n';
+    if (ShouldPrintNewline)
+      Os << '\n';
   }
   void flushEol() {
     if (HasPendingNewline) {
diff --git a/mlir/lib/Rewrite/PatternApplicator.cpp b/mlir/lib/Rewrite/PatternApplicator.cpp
index b2b372b7b1249..e13bcff2767ec 100644
--- a/mlir/lib/Rewrite/PatternApplicator.cpp
+++ b/mlir/lib/Rewrite/PatternApplicator.cpp
@@ -13,7 +13,7 @@
 
 #include "mlir/Rewrite/PatternApplicator.h"
 #include "ByteCode.h"
-#include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 
 #ifndef NDEBUG
 #include "llvm/ADT/ScopeExit.h"
@@ -51,9 +51,7 @@ static Operation *getDumpRootOp(Operation *op) {
   return op;
 }
 static void logSucessfulPatternApplication(Operation *op) {
-  llvm::dbgs() << "// *** IR Dump After Pattern Application ***\n";
-  op->dump();
-  llvm::dbgs() << "\n\n";
+  LDBG(2) << "// *** IR Dump After Pattern Application ***\n" << *op << "\n";
 }
 #endif
 
@@ -208,8 +206,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
             result =
                 bytecode->rewrite(rewriter, *pdlMatch, *mutableByteCodeState);
           } else {
-            LLVM_DEBUG(llvm::dbgs() << "Trying to match \""
-                                    << bestPattern->getDebugName() << "\"\n");
+            LDBG() << "Trying to match \"" << bestPattern->getDebugName()
+                   << "\"";
             const auto *pattern =
                 static_cast<const RewritePattern *>(bestPattern);
 
@@ -223,9 +221,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
                 [&] { rewriter.setListener(oldListener); });
 #endif
             result = pattern->matchAndRewrite(op, rewriter);
-            LLVM_DEBUG(llvm::dbgs()
-                       << "\"" << bestPattern->getDebugName() << "\" result "
-                       << succeeded(result) << "\n");
+            LDBG() << " -> matchAndRewrite "
+                   << (succeeded(result) ? "successful" : "failed");
           }
 
           // Process the result of the pattern application.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 08803e082b057..abd75558e290f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -1129,8 +1130,14 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
   /// verification.
   SmallPtrSet<Operation *, 1> pendingRootUpdates;
 
+  /// A raw output stream used to prefix the debug log.
+  llvm::impl::raw_ldbg_ostream os{(Twine("[") + DEBUG_TYPE + "] ").str(),
+                                  llvm::dbgs(), /*HasPendingNewline=*/false,
+                                  /*ShouldPrintNewline=*/false};
+
   /// A logger used to emit diagnostics during the conversion process.
-  llvm::ScopedPrinter logger{llvm::dbgs()};
+  llvm::ScopedPrinter logger{os};
+  std::string logPrefix;
 #endif
 };
 } // namespace detail

@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

Changes

This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the raw_ldbg_ostream to consistently prefix each new line.


Full diff: https://github.com/llvm/llvm-project/pull/150991.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Support/DebugLog.h (+9-4)
  • (modified) mlir/lib/Rewrite/PatternApplicator.cpp (+6-9)
  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+8-1)
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 8fca2d5e2816b..1b4fe9d13030f 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -85,7 +85,8 @@ namespace impl {
 class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   std::string Prefix;
   raw_ostream &Os;
-  bool HasPendingNewline = true;
+  bool HasPendingNewline;
+  bool ShouldPrintNewline;
 
   /// Split the line on newlines and insert the prefix before each newline.
   /// Forward everything to the underlying stream.
@@ -117,13 +118,17 @@ class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
   }
 
 public:
-  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os)
-      : Prefix(std::move(Prefix)), Os(Os) {
+  explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os,
+                            bool HasPendingNewline = true,
+                            bool ShouldPrintNewline = true)
+      : Prefix(std::move(Prefix)), Os(Os), HasPendingNewline(HasPendingNewline),
+        ShouldPrintNewline(ShouldPrintNewline) {
     SetUnbuffered();
   }
   ~raw_ldbg_ostream() final {
     flushEol();
-    Os << '\n';
+    if (ShouldPrintNewline)
+      Os << '\n';
   }
   void flushEol() {
     if (HasPendingNewline) {
diff --git a/mlir/lib/Rewrite/PatternApplicator.cpp b/mlir/lib/Rewrite/PatternApplicator.cpp
index b2b372b7b1249..e13bcff2767ec 100644
--- a/mlir/lib/Rewrite/PatternApplicator.cpp
+++ b/mlir/lib/Rewrite/PatternApplicator.cpp
@@ -13,7 +13,7 @@
 
 #include "mlir/Rewrite/PatternApplicator.h"
 #include "ByteCode.h"
-#include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 
 #ifndef NDEBUG
 #include "llvm/ADT/ScopeExit.h"
@@ -51,9 +51,7 @@ static Operation *getDumpRootOp(Operation *op) {
   return op;
 }
 static void logSucessfulPatternApplication(Operation *op) {
-  llvm::dbgs() << "// *** IR Dump After Pattern Application ***\n";
-  op->dump();
-  llvm::dbgs() << "\n\n";
+  LDBG(2) << "// *** IR Dump After Pattern Application ***\n" << *op << "\n";
 }
 #endif
 
@@ -208,8 +206,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
             result =
                 bytecode->rewrite(rewriter, *pdlMatch, *mutableByteCodeState);
           } else {
-            LLVM_DEBUG(llvm::dbgs() << "Trying to match \""
-                                    << bestPattern->getDebugName() << "\"\n");
+            LDBG() << "Trying to match \"" << bestPattern->getDebugName()
+                   << "\"";
             const auto *pattern =
                 static_cast<const RewritePattern *>(bestPattern);
 
@@ -223,9 +221,8 @@ LogicalResult PatternApplicator::matchAndRewrite(
                 [&] { rewriter.setListener(oldListener); });
 #endif
             result = pattern->matchAndRewrite(op, rewriter);
-            LLVM_DEBUG(llvm::dbgs()
-                       << "\"" << bestPattern->getDebugName() << "\" result "
-                       << succeeded(result) << "\n");
+            LDBG() << " -> matchAndRewrite "
+                   << (succeeded(result) ? "successful" : "failed");
           }
 
           // Process the result of the pattern application.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 08803e082b057..abd75558e290f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugLog.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -1129,8 +1130,14 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
   /// verification.
   SmallPtrSet<Operation *, 1> pendingRootUpdates;
 
+  /// A raw output stream used to prefix the debug log.
+  llvm::impl::raw_ldbg_ostream os{(Twine("[") + DEBUG_TYPE + "] ").str(),
+                                  llvm::dbgs(), /*HasPendingNewline=*/false,
+                                  /*ShouldPrintNewline=*/false};
+
   /// A logger used to emit diagnostics during the conversion process.
-  llvm::ScopedPrinter logger{llvm::dbgs()};
+  llvm::ScopedPrinter logger{os};
+  std::string logPrefix;
 #endif
 };
 } // namespace detail

Copy link
Member

@jpienaar jpienaar left a comment

Choose a reason for hiding this comment

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

Why was pending newline not needed for the dialect one? (Potentially as it already has it in the logging?)

@joker-eph
Copy link
Collaborator Author

This is because the use of raw_ldbg_ostream directly is not guarded by DEBUG_TYPE, it's unconditionally constructed and destructed once (not once per log). Since we removed the class LogWithNewLine and merged it with the stream, the stream itself is adding a \n on construction (before that is was done in the LogWithNewLine destructor).

@joker-eph joker-eph force-pushed the dialect_conversion_ldbg branch 2 times, most recently from 8d93fce to 3a85411 Compare July 31, 2025 14:06
@joker-eph joker-eph requested a review from jpienaar July 31, 2025 14:06
@joker-eph joker-eph closed this Jul 31, 2025
@joker-eph joker-eph force-pushed the dialect_conversion_ldbg branch from 3a85411 to 9a9b8b7 Compare July 31, 2025 16:20
@joker-eph joker-eph reopened this Jul 31, 2025
@joker-eph joker-eph force-pushed the dialect_conversion_ldbg branch from 3a85411 to 485e231 Compare July 31, 2025 18:59
…ogging format

This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the raw_ldbg_ostream to
consistently prefix each new line.
@joker-eph joker-eph force-pushed the dialect_conversion_ldbg branch from 485e231 to 2a35d8d Compare August 1, 2025 10:51
@joker-eph joker-eph merged commit c569c1f into llvm:main Aug 1, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:support mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants