Refactor the mlir-opt command line options related to debugging in a helper

[mehdi_amini]

Summary

This makes it reusable across various tooling and reduces the amount of
boilerplate needed.

[mehdi_amini]

rebase

[Mogball]

LGTM

mlir/lib/Debug/CLOptionsSetup.cpp

61–66

[rriddle]

mlir/include/mlir/Debug/CLOptionsSetup.h

31–33

35

Is there a better name than GDB? Would remove a lot of questions of "what about lldb?"

[mehdi_amini]

mlir/include/mlir/Debug/CLOptionsSetup.h

35

I don't know, I tend to use "gdb" for the debugger, but what about simply "Debugger"?

[mehdi_amini]

rebase

mlir/include/mlir/Debug/CLOptionsSetup.h

+//===- CLOptionsSetup.h - Helpers to setup debug CL options -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_DEBUG_CLOPTIONSSETUP_H
+#define MLIR_DEBUG_CLOPTIONSSETUP_H
+
+#include "mlir/Debug/BreakpointManagers/FileLineColLocBreakpointManager.h"
+#include "mlir/Support/LogicalResult.h"
+#include "llvm/ADT/StringRef.h"
+
+#include <memory>
+
+namespace mlir {
+class MLIRContext;
+namespace tracing {
+class BreakpointManager;
+
+class DebugConfig {
+public:
+  /// Register the options as global LLVM command line options.
+  static void registerCLOptions();
+
+  /// Create a new config with the default set from the CL options.
+  static DebugConfig createFromCLOptions();
+
+  ///
+  /// Options.
+  ///
+
+  /// Enable the gdb action hook: it makes GDB able to intercept MLIR Actions.
+  void enableGdbActionHook(bool enabled = true) {
+    enableGdbActionHookFlag = enabled;
+  }
+
+  /// Return true if the gdb action hook is enabled.
+  bool isGdbActionHookEnabled() const { return enableGdbActionHookFlag; }
+
+  /// Set the filename to use for logging actions, use "-" for stdout.
+  DebugConfig &logActionsTo(StringRef filename) {
+    logActionsToFlag = filename;
+    return *this;
+  }
+  /// Get the filename to use for logging actions.
+  StringRef getLogActionsTo() const { return logActionsToFlag; }
+
+  /// Set the location breakpoints to use for logging actions.
+  /// Ownership stays with the caller.
+  DebugConfig &
+  addLogActionLocFilter(tracing::BreakpointManager *breakpointManager) {
+    logActionLocationFilter.push_back(breakpointManager);
+    return *this;
+  }
+
+  /// Get the location breakpoints to use for logging actions.
+  ArrayRef<tracing::BreakpointManager *> getLogActionsLocFilters() const {
+    return logActionLocationFilter;
+  }
+
+protected:
+  /// Enable the gdb action hook: GDB can intercept MLIR Actions.
+  bool enableGdbActionHookFlag = false;
+
+  /// Log action execution to the given file (or "-" for stdout)
+  std::string logActionsToFlag;
+
+  /// Location Breakpoints to filter the action logging.
+  std::vector<tracing::BreakpointManager *> logActionLocationFilter;
+};
+
+/// This is a RAII class that installs the debug handlers on the context
+/// based on the provided configuration.
+class InstallDebugHandler {
+public:
+  InstallDebugHandler(MLIRContext &context, const DebugConfig &config);
+  ~InstallDebugHandler();
+
+private:
+  class Impl;
+  std::unique_ptr<Impl> impl;
+};
+
+} // namespace tracing
+} // namespace mlir
+
+#endif // MLIR_DEBUG_CLOPTIONSSETUP_H

mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h

 //===- MlirOptMain.h - MLIR Optimizer Driver main ---------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Main entry function for mlir-opt for when built as standalone binary.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_TOOLS_MLIROPT_MLIROPTMAIN_H
 #define MLIR_TOOLS_MLIROPT_MLIROPTMAIN_H
 
-#include "mlir/Debug/BreakpointManagers/FileLineColLocBreakpointManager.h"
+#include "mlir/Debug/CLOptionsSetup.h"
 #include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/StringRef.h"
 
 #include <cstdlib>
 #include <functional>
 #include <memory>
 
 namespace llvm {
 class raw_ostream;
 class MemoryBuffer;
 } // namespace llvm
 
 namespace mlir {
 class DialectRegistry;
 class PassPipelineCLParser;
 class PassManager;
-namespace tracing {
-class FileLineColLocBreakpointManager;
-}
 
 /// Configuration options for the mlir-opt tool.
 /// This is intended to help building tools like mlir-opt by collecting the
 /// supported options.
 /// The API is fluent, and the options are sorted in alphabetical order below.
 /// The options can be exposed to the LLVM command line by registering them
 /// with `MlirOptMainConfig::registerCLOptions();` and creating a config using
 /// `auto config = MlirOptMainConfig::createFromCLOptions();`.
public:
   MlirOptMainConfig &allowUnregisteredDialects(bool allow) {
     allowUnregisteredDialectsFlag = allow;
     return *this;
   }
   bool shouldAllowUnregisteredDialects() const {
     return allowUnregisteredDialectsFlag;
   }
 
+  /// Set the debug configuration to use.
+  MlirOptMainConfig &setDebugConfig(tracing::DebugConfig config) {
+    debugConfig = std::move(config);
+    return *this;
+  }
+  tracing::DebugConfig &getDebugConfig() { return debugConfig; }
+  const tracing::DebugConfig &getDebugConfig() const { return debugConfig; }
+
   /// Print the pass-pipeline as text before executing.
   MlirOptMainConfig &dumpPassPipeline(bool dump) {
     dumpPassPipelineFlag = dump;
     return *this;
   }
   bool shouldDumpPassPipeline() const { return dumpPassPipelineFlag; }
 
   /// Set the output format to bytecode instead of textual IR.
   MlirOptMainConfig &emitBytecode(bool emit) {
     emitBytecodeFlag = emit;
     return *this;
   }
   bool shouldEmitBytecode() const { return emitBytecodeFlag; }
 
-  /// Enable the gdb action hook: it makes GDB able to intercept MLIR Actions.
-  void enableGdbActionHook(bool enabled = true) {
-    enableGdbActionHookFlag = enabled;
-  }
-
-  /// Return true if the gdb action hook is enabled.
-  bool isGdbActionHookEnabled() const { return enableGdbActionHookFlag; }
-
-  /// Set the filename to use for logging actions, use "-" for stdout.
-  MlirOptMainConfig &logActionsTo(StringRef filename) {
-    logActionsToFlag = filename;
-    return *this;
-  }
-  /// Get the filename to use for logging actions.
-  StringRef getLogActionsTo() const { return logActionsToFlag; }
-
-  /// Set the location breakpoints to use for logging actions.
-  /// Ownership stays with the caller.
-  void addLogActionLocFilter(tracing::BreakpointManager *breakpointManager) {
-    logActionLocationFilter.push_back(breakpointManager);
-  }
-
-  /// Get the location breakpoints to use for logging actions.
-  ArrayRef<tracing::BreakpointManager *> getLogActionsLocFilters() const {
-    return logActionLocationFilter;
-  }
-
   /// Set the callback to populate the pass manager.
   MlirOptMainConfig &
   setPassPipelineSetupFn(std::function<LogicalResult(PassManager &)> callback) {
     passPipelineCallback = std::move(callback);
     return *this;
   }
 
   /// Set the parser to use to populate the pass manager.
public:
   bool shouldVerifyPasses() const { return verifyPassesFlag; }
 
 protected:
   /// Allow operation with no registered dialects.
   /// This option is for convenience during testing only and discouraged in
   /// general.
   bool allowUnregisteredDialectsFlag = false;
 
+  /// Configuration for the debugging hooks.
+  tracing::DebugConfig debugConfig;
+
   /// Print the pipeline that will be run.
   bool dumpPassPipelineFlag = false;
 
   /// Emit bytecode instead of textual assembly when generating output.
   bool emitBytecodeFlag = false;
 
   /// Enable the gdb action hook: GDB can intercept MLIR Actions.
   bool enableGdbActionHookFlag = false;
 
-  /// Log action execution to the given file (or "-" for stdout)
-  std::string logActionsToFlag;
-
-  /// Location Breakpoints to filter the action logging.
-  std::vector<tracing::BreakpointManager *> logActionLocationFilter;
-
   /// The callback to populate the pass manager.
   std::function<LogicalResult(PassManager &)> passPipelineCallback;
 
   /// Deprecated.
   bool preloadDialectsInContextFlag = false;
 
   /// Show the registered dialects before trying to load the input file.
   bool showDialectsFlag = false;

mlir/lib/Debug/CLOptionsSetup.cpp

+//===- CLOptionsSetup.cpp - Helpers to setup debug CL options ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Debug/CLOptionsSetup.h"
+
+#include "mlir/Debug/Counter.h"
+#include "mlir/Debug/ExecutionContext.h"
+#include "mlir/Debug/GdbDebugExecutionContextHook.h"
+#include "mlir/Debug/Observers/ActionLogging.h"
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/Support/FileUtilities.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ToolOutputFile.h"
+
+using namespace mlir;
+using namespace mlir::tracing;
+using namespace llvm;
+
+namespace {
+struct DebugConfigCLOptions : public DebugConfig {
+  DebugConfigCLOptions() {
+    static cl::opt<std::string, /*ExternalStorage=*/true> logActionsTo{
+        "log-actions-to",
+        cl::desc("Log action execution to a file, or stderr if "
+                 " '-' is passed"),
+        cl::location(logActionsToFlag)};
+
+    static cl::list<std::string> logActionLocationFilter(
+        "log-mlir-actions-filter",
+        cl::desc(
+            "Comma separated list of locations to filter actions from logging"),
+        cl::CommaSeparated,
+        cl::cb<void, std::string>([&](const std::string &location) {
+          static bool register_once = [&] {
+            addLogActionLocFilter(&locBreakpointManager);
+            return true;
+          }();
+          (void)register_once;
+          static std::vector<std::string> locations;
+          locations.push_back(location);
+          StringRef locStr = locations.back();
+
+          // Parse the individual location filters and set the breakpoints.
+          auto diag = [](StringRef msg) { errs() << msg << "\n"; };
+          auto locBreakpoint =
+              tracing::FileLineColLocBreakpoint::parseFromString(locStr, diag);
+          if (failed(locBreakpoint)) {
+            errs() << "Invalid location filter: " << locStr << "\n";
+            exit(1);
+          }
+          auto [file, line, col] = *locBreakpoint;
+          locBreakpointManager.addBreakpoint(file, line, col);
+        }));
+  }
+  tracing::FileLineColLocBreakpointManager locBreakpointManager;
+};
+
+ManagedStatic<DebugConfigCLOptions> clOptionsConfig;
+
+} // namespace
+
+void DebugConfig::registerCLOptions() { *clOptionsConfig; }
+
+DebugConfig DebugConfig::createFromCLOptions() { return *clOptionsConfig; }
+
+class InstallDebugHandler::Impl {
+public:
+  Impl(MLIRContext &context, const DebugConfig &config) {
+    if (config.getLogActionsTo().empty() && !config.isGdbActionHookEnabled()) {
+      if (tracing::DebugCounter::isDebugCounterActivated())
+        context.registerActionHandler(tracing::DebugCounter{});
+      return;
+    }
+    errs() << "ExecutionContext registered on the context";
+    if (tracing::DebugCounter::isDebugCounterActivated())
+      emitError(UnknownLoc::get(&context),
+                "Debug counters are incompatible with --log-actions-to and "
+                "--mlir-enable-gdb-hook options and are disabled");
+    if (!config.getLogActionsTo().empty()) {
+      std::string errorMessage;
+      logActionsFile = openOutputFile(config.getLogActionsTo(), &errorMessage);
+      if (!logActionsFile) {
+        errs() << "Error handling --log-actions-to: " << errorMessage << "\n";
+        return;
+      }
+      logActionsFile->keep();
+      raw_fd_ostream &logActionsStream = logActionsFile->os();
+      actionLogger = std::make_unique<tracing::ActionLogger>(logActionsStream);
+      for (const auto *locationBreakpoint : config.getLogActionsLocFilters())
+        actionLogger->addBreakpointManager(locationBreakpoint);
+      executionContext.registerObserver(actionLogger.get());
+    }
+    if (config.isGdbActionHookEnabled()) {
+      errs() << " (with GDB hook)";
+      setupGdbDebugExecutionContextHook(executionContext);
+    }
+    errs() << "\n";
+    context.registerActionHandler(executionContext);
+  }
+
+private:
+  std::unique_ptr<ToolOutputFile> logActionsFile;
+  tracing::ExecutionContext executionContext;
+  std::unique_ptr<tracing::ActionLogger> actionLogger;
+  std::vector<std::unique_ptr<tracing::FileLineColLocBreakpoint>>
+      locationBreakpoints;
+};
+
+InstallDebugHandler::InstallDebugHandler(MLIRContext &context,
+                                         const DebugConfig &config)
+    : impl(std::make_unique<Impl>(context, config)) {}
+
+InstallDebugHandler::~InstallDebugHandler() = default;

mlir/lib/Debug/CMakeLists.txt

 add_subdirectory(Observers)
 
 add_mlir_library(MLIRDebug
+  CLOptionsSetup.cpp
   DebugCounter.cpp
   ExecutionContext.cpp
   BreakpointManagers/FileLineColLocBreakpointManager.cpp
   GdbDebugExecutionContextHook.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Debug
 
   LINK_LIBS PUBLIC
   ${LLVM_PTHREAD_LIB}
   MLIRIR
   MLIRSupport
 )
 

mlir/lib/Tools/mlir-opt/MlirOptMain.cpp

 //===- MlirOptMain.cpp - MLIR Optimizer Driver ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This is a utility that runs an optimization pass and prints the result back
 // out. It is designed to support unit testing.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Tools/mlir-opt/MlirOptMain.h"
 #include "mlir/Bytecode/BytecodeWriter.h"
+#include "mlir/Debug/CLOptionsSetup.h"
 #include "mlir/Debug/Counter.h"
 #include "mlir/Debug/ExecutionContext.h"
 #include "mlir/Debug/GdbDebugExecutionContextHook.h"
 #include "mlir/Debug/Observers/ActionLogging.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/Diagnostics.h"
static cl::opt<bool, /*ExternalStorage=*/true> enableGdbHook(
         cl::location(enableGdbActionHookFlag), cl::init(false));
 
     static cl::opt<bool, /*ExternalStorage=*/true> explicitModule(
         "no-implicit-module",
         cl::desc("Disable implicit addition of a top-level module op during "
                  "parsing"),
         cl::location(useExplicitModuleFlag), cl::init(false));
 
-    static cl::opt<std::string, /*ExternalStorage=*/true> logActionsTo{
-        "log-actions-to",
-        cl::desc("Log action execution to a file, or stderr if "
-                 " '-' is passed"),
-        cl::location(logActionsToFlag)};
-
-    static cl::list<std::string> logActionLocationFilter(
-        "log-mlir-actions-filter",
-        cl::desc(
-            "Comma separated list of locations to filter actions from logging"),
-        cl::CommaSeparated,
-        cl::cb<void, std::string>([&](const std::string &location) {
-          static bool register_once = [&] {
-            addLogActionLocFilter(&locBreakpointManager);
-            return true;
-          }();
-          (void)register_once;
-          static std::vector<std::string> locations;
-          locations.push_back(location);
-          StringRef locStr = locations.back();
-
-          // Parse the individual location filters and set the breakpoints.
-          auto diag = [](StringRef msg) { llvm::errs() << msg << "\n"; };
-          auto locBreakpoint =
-              tracing::FileLineColLocBreakpoint::parseFromString(locStr, diag);
-          if (failed(locBreakpoint)) {
-            llvm::errs() << "Invalid location filter: " << locStr << "\n";
-            exit(1);
-          }
-          auto [file, line, col] = *locBreakpoint;
-          locBreakpointManager.addBreakpoint(file, line, col);
-        }));
-
     static cl::opt<bool, /*ExternalStorage=*/true> showDialects(
         "show-dialects",
         cl::desc("Print the list of registered dialects and exit"),
         cl::location(showDialectsFlag), cl::init(false));
 
     static cl::opt<bool, /*ExternalStorage=*/true> splitInputFile(
         "split-input-file",
         cl::desc("Split the input file into pieces and process each "
MlirOptMainConfigCLOptions() {
     static cl::opt<bool, /*ExternalStorage=*/true> verifyPasses(
         "verify-each",
         cl::desc("Run the verifier after each transformation pass"),
         cl::location(verifyPassesFlag), cl::init(true));
 
     static PassPipelineCLParser passPipeline("", "Compiler passes to run", "p");
     setPassPipelineParser(passPipeline);
   }
-
-  tracing::FileLineColLocBreakpointManager locBreakpointManager;
 };
 } // namespace
 
 ManagedStatic<MlirOptMainConfigCLOptions> clOptionsConfig;
 
-void MlirOptMainConfig::registerCLOptions() { *clOptionsConfig; }
+void MlirOptMainConfig::registerCLOptions() {
+  *clOptionsConfig;
+  tracing::DebugConfig::registerCLOptions();
+}
 
 MlirOptMainConfig MlirOptMainConfig::createFromCLOptions() {
+  clOptionsConfig->setDebugConfig(tracing::DebugConfig::createFromCLOptions());
   return *clOptionsConfig;
 }
 
 MlirOptMainConfig &MlirOptMainConfig::setPassPipelineParser(
     const PassPipelineCLParser &passPipeline) {
   passPipelineCallback = [&](PassManager &pm) {
     auto errorHandler = [&](const Twine &msg) {
       emitError(UnknownLoc::get(pm.getContext())) << msg;
       return failure();
     };
     if (failed(passPipeline.addToPipeline(pm, errorHandler)))
       return failure();
     if (this->shouldDumpPassPipeline()) {
 
       pm.dump();
       llvm::errs() << "\n";
     }
     return success();
   };
   return *this;
 }
 
-/// Set the ExecutionContext on the context and handle the observers.
-class InstallDebugHandler {
-public:
-  InstallDebugHandler(MLIRContext &context, const MlirOptMainConfig &config) {
-    if (config.getLogActionsTo().empty() && !config.isGdbActionHookEnabled()) {
-      if (tracing::DebugCounter::isDebugCounterActivated())
-        context.registerActionHandler(tracing::DebugCounter{});
-      return;
-    }
-    llvm::errs() << "ExecutionContext registered on the context";
-    if (tracing::DebugCounter::isDebugCounterActivated())
-      emitError(UnknownLoc::get(&context),
-                "Debug counters are incompatible with --log-actions-to and "
-                "--mlir-enable-gdb-hook options and are disabled");
-    if (!config.getLogActionsTo().empty()) {
-      std::string errorMessage;
-      logActionsFile = openOutputFile(config.getLogActionsTo(), &errorMessage);
-      if (!logActionsFile) {
-        llvm::errs() << "Error handling --log-actions-to: " << errorMessage
-                     << "\n";
-        return;
-      }
-      logActionsFile->keep();
-      raw_fd_ostream &logActionsStream = logActionsFile->os();
-      actionLogger = std::make_unique<tracing::ActionLogger>(logActionsStream);
-      for (const auto *locationBreakpoint : config.getLogActionsLocFilters())
-        actionLogger->addBreakpointManager(locationBreakpoint);
-      executionContext.registerObserver(actionLogger.get());
-    }
-    if (config.isGdbActionHookEnabled()) {
-      llvm::errs() << " (with GDB hook)";
-      setupGdbDebugExecutionContextHook(executionContext);
-    }
-    llvm::errs() << "\n";
-    context.registerActionHandler(executionContext);
-  }
-
-private:
-  std::unique_ptr<llvm::ToolOutputFile> logActionsFile;
-  tracing::ExecutionContext executionContext;
-  std::unique_ptr<tracing::ActionLogger> actionLogger;
-  std::vector<std::unique_ptr<tracing::FileLineColLocBreakpoint>>
-      locationBreakpoints;
-};
-
 /// Perform the actions on the input file indicated by the command line flags
 /// within the specified context.
 ///
 /// This typically parses the main source file, runs zero or more optimization
 /// passes, then prints the output.
 ///
 static LogicalResult
 performActions(raw_ostream &os,
static LogicalResult processBuffer(raw_ostream &os,
 
   // Parse the input file.
   if (config.shouldPreloadDialectsInContext())
     context.loadAllAvailableDialects();
   context.allowUnregisteredDialects(config.shouldAllowUnregisteredDialects());
   if (config.shouldVerifyDiagnostics())
     context.printOpOnDiagnostic(false);
 
-  InstallDebugHandler installDebugHandler{context, config};
+  tracing::InstallDebugHandler installDebugHandler(context,
+                                                   config.getDebugConfig());
 
   // If we are in verify diagnostics mode then we have a lot of work to do,
   // otherwise just perform the actions without worrying about it.
   if (!config.shouldVerifyDiagnostics()) {
     SourceMgrDiagnosticHandler sourceMgrHandler(*sourceMgr, &context);
     return performActions(os, sourceMgr, &context, config);
   }