[MLIR] Add debug log to the pass manager (NFC) #156205
Merged
+214
−30
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-support Author: Mehdi Amini (joker-eph) ChangesPatch is 21.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156205.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index dce706e196bde..bbb4a02066951 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -76,6 +76,13 @@ namespace llvm {
#define __LLVM_FILE_NAME__ ::llvm::impl::getShortFileName(__FILE__)
#endif
+#define LDBG_OS(name, LEVEL) \
+ ::llvm::impl::raw_ldbg_ostream name { \
+ ::llvm::impl::computePrefix(DEBUG_TYPE, __LLVM_FILE_NAME__, __LINE__, \
+ LEVEL), \
+ llvm::dbgs() \
+ }
+
#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, \
LINE) \
for (bool _c = \
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 7094c8e279f2d..9b2491c185868 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -21,11 +21,14 @@
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/DebugLog.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/Threading.h"
#include <optional>
+#define DEBUG_TYPE "pass-manager"
+
using namespace mlir;
using namespace mlir::detail;
@@ -242,6 +245,7 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
};
// Walk the pass list and merge adjacent adaptors.
+ LDBG(3) << "Merging adjacent adaptors in pass list";
OpToOpPassAdaptor *lastAdaptor = nullptr;
for (auto &pass : passes) {
// Check to see if this pass is an adaptor.
@@ -249,18 +253,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// If it is the first adaptor in a possible chain, remember it and
// continue.
if (!lastAdaptor) {
+ LDBG(3) << "Found first adaptor in chain";
lastAdaptor = currentAdaptor;
continue;
}
// Otherwise, try to merge into the existing adaptor and delete the
// current one. If merging fails, just remember this as the last adaptor.
- if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor)))
+ LDBG(3) << "Attempting to merge adaptor with "
+ << currentAdaptor->getPassManagers().size()
+ << " managers into previous adaptor";
+ if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor))) {
+ LDBG(3) << "Successfully merged adaptors, removing current one";
pass.reset();
- else
+ } else {
+ LDBG(3) << "Failed to merge adaptors, keeping current as last";
lastAdaptor = currentAdaptor;
+ }
} else if (lastAdaptor) {
// If this pass isn't an adaptor, finalize it and forget the last adaptor.
+ LDBG(3) << "Finalizing adaptor chain before non-adaptor pass";
if (failed(finalizeAdaptor(lastAdaptor)))
return failure();
lastAdaptor = nullptr;
@@ -273,15 +285,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// Now that the adaptors have been merged, erase any empty slots corresponding
// to the merged adaptors that were nulled-out in the loop above.
+ size_t beforeErase = passes.size();
llvm::erase_if(passes, std::logical_not<std::unique_ptr<Pass>>());
+ if (beforeErase != passes.size()) {
+ LDBG(3) << "Removed " << (beforeErase - passes.size())
+ << " merged adaptor slots from pass list";
+ }
// If this is a op-agnostic pass manager, there is nothing left to do.
std::optional<OperationName> rawOpName = getOpName(*ctx);
- if (!rawOpName)
+ if (!rawOpName) {
+ LDBG(3)
+ << "Op-agnostic pass manager, skipping operation-specific verification";
return success();
+ }
// Otherwise, verify that all of the passes are valid for the current
// operation anchor.
+ LDBG(3) << "Verifying " << passes.size() << " passes for operation '"
+ << getOpAnchorName() << "'";
+
std::optional<RegisteredOperationName> opName =
rawOpName->getRegisteredInfo();
for (std::unique_ptr<Pass> &pass : passes) {
@@ -292,6 +315,8 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
<< "'!";
}
}
+
+ LDBG(3) << "Pass list finalization completed successfully";
return success();
}
@@ -456,23 +481,44 @@ OpPassManager::Nesting OpPassManager::getNesting() { return impl->nesting; }
LogicalResult OpPassManager::initialize(MLIRContext *context,
unsigned newInitGeneration) {
- if (impl->initializationGeneration == newInitGeneration)
+
+ if (impl->initializationGeneration == newInitGeneration) {
+ LDBG(2) << "Pass manager already initialized "
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
return success();
+ }
+
+ LDBG(2) << "Initializing pass manager '" << getOpAnchorName()
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
impl->initializationGeneration = newInitGeneration;
+
for (Pass &pass : getPasses()) {
// If this pass isn't an adaptor, directly initialize it.
auto *adaptor = dyn_cast<OpToOpPassAdaptor>(&pass);
if (!adaptor) {
- if (failed(pass.initialize(context)))
+ LDBG(2) << "Initializing pass '" << pass.getName() << "'";
+ if (failed(pass.initialize(context))) {
+ LDBG(2) << "Failed to initialize pass '" << pass.getName() << "'";
return failure();
+ }
continue;
}
// Otherwise, initialize each of the adaptors pass managers.
+ LDBG(3) << "Initializing adaptor pass with "
+ << adaptor->getPassManagers().size() << " nested managers";
for (OpPassManager &adaptorPM : adaptor->getPassManagers())
- if (failed(adaptorPM.initialize(context, newInitGeneration)))
+ if (failed(adaptorPM.initialize(context, newInitGeneration))) {
+ LDBG(2) << "Failed to initialize nested pass manager";
return failure();
+ }
}
+
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Pass manager initialization completed successfully: ";
+ printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
return success();
}
@@ -499,16 +545,23 @@ llvm::hash_code OpPassManager::hash() {
LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration) {
+ LDBG() << "Running pass '" << pass->getName() << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' at "
+ << op->getLoc();
+
std::optional<RegisteredOperationName> opInfo = op->getRegisteredInfo();
- if (!opInfo)
+ if (!opInfo) {
return op->emitOpError()
<< "trying to schedule a pass on an unregistered operation";
- if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>())
+ }
+ if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>()) {
return op->emitOpError() << "trying to schedule a pass on an operation not "
"marked as 'IsolatedFromAbove'";
- if (!pass->canScheduleOn(*op->getName().getRegisteredInfo()))
+ }
+ if (!pass->canScheduleOn(*op->getName().getRegisteredInfo())) {
return op->emitOpError()
<< "trying to schedule a pass on an unsupported operation";
+ }
// Initialize the pass state with a callback for the pass to dynamically
// execute a pipeline on the currently visited operation.
@@ -526,8 +579,10 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
pipeline.getImpl().canScheduleOn(*op->getContext(), root->getName()));
// Before running, finalize the passes held by the pipeline.
- if (failed(pipeline.getImpl().finalizePassList(root->getContext())))
+ if (failed(pipeline.getImpl().finalizePassList(root->getContext()))) {
+ LDBG() << "Failed to finalize pass list for pipeline";
return failure();
+ }
// Initialize the user provided pipeline and execute the pipeline.
if (failed(pipeline.initialize(root->getContext(), parentInitGeneration)))
@@ -599,6 +654,11 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
OpPassManager &pm, Operation *op, AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration, PassInstrumentor *instrumentor,
const PassInstrumentation::PipelineParentInfo *parentInfo) {
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Running pipeline on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << pm.size() << " passes, verifyPasses=" << verifyPasses;
+ pm.printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
assert((!instrumentor || parentInfo) &&
"expected parent info if instrumentor is provided");
auto scopeExit = llvm::make_scope_exit([&] {
@@ -615,9 +675,14 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
*parentInfo);
}
- for (Pass &pass : pm.getPasses())
- if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration)))
+ for (Pass &pass : pm.getPasses()) {
+ if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration))) {
+ LDBG() << "Pipeline failed for pass '" << pass.getName()
+ << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "'";
return failure();
+ }
+ }
if (instrumentor) {
instrumentor->runAfterPipeline(pm.getOpName(*op->getContext()),
@@ -630,9 +695,19 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
/// does not exist.
static OpPassManager *
findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
+ LDBG(3) << "Looking for pass manager with anchor name '" << name << "' among "
+ << mgrs.size() << " managers";
+
auto *it = llvm::find_if(
mgrs, [&](OpPassManager &mgr) { return mgr.getOpAnchorName() == name; });
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(2) << "No pass manager found with anchor name '" << name << "'";
+ return nullptr;
+ }
+
+ LDBG(2) << "Found pass manager with anchor name '" << name << "'";
+ return &*it;
}
/// Find an operation pass manager that can operate on an operation of the given
@@ -640,10 +715,22 @@ findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
static OpPassManager *findPassManagerFor(MutableArrayRef<OpPassManager> mgrs,
OperationName name,
MLIRContext &context) {
+ LDBG(4) << "Looking for pass manager that can handle operation '" << name
+ << "' among " << mgrs.size() << " managers";
+
auto *it = llvm::find_if(mgrs, [&](OpPassManager &mgr) {
return mgr.getImpl().canScheduleOn(context, name);
});
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(4) << "No pass manager found that can handle operation '" << name
+ << "'";
+ return nullptr;
+ }
+
+ LDBG(4) << "Found pass manager '" << it->getOpAnchorName()
+ << "' that can handle operation '" << name << "'";
+ return &*it;
}
OpToOpPassAdaptor::OpToOpPassAdaptor(OpPassManager &&mgr) {
@@ -657,6 +744,9 @@ void OpToOpPassAdaptor::getDependentDialects(DialectRegistry &dialects) const {
LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
OpToOpPassAdaptor &rhs) {
+ LDBG(3) << "Attempting to merge pass adaptor with " << mgrs.size()
+ << " managers into rhs with " << rhs.mgrs.size() << " managers";
+
// Functor used to check if a pass manager is generic, i.e. op-agnostic.
auto isGenericPM = [&](OpPassManager &pm) { return !pm.getOpName(); };
@@ -682,14 +772,24 @@ LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
//
// Check the current adaptor.
auto *lhsGenericPMIt = llvm::find_if(mgrs, isGenericPM);
- if (lhsGenericPMIt != mgrs.end() &&
- hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs))
- return failure();
+ if (lhsGenericPMIt != mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on LHS, checking for conflicts";
+ if (hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs)) {
+ LDBG(4)
+ << "Merge failed: LHS generic pass manager has conflicts with RHS";
+ return failure();
+ }
+ }
// Check the rhs adaptor.
auto *rhsGenericPMIt = llvm::find_if(rhs.mgrs, isGenericPM);
- if (rhsGenericPMIt != rhs.mgrs.end() &&
- hasScheduleConflictWith(*rhsGenericPMIt, mgrs))
- return failure();
+ if (rhsGenericPMIt != rhs.mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on RHS, checking for conflicts";
+ if (hasScheduleConflictWith(*rhsGenericPMIt, mgrs)) {
+ LDBG(4)
+ << "Merge failed: RHS generic pass manager has conflicts with LHS";
+ return failure();
+ }
+ }
for (auto &pm : mgrs) {
// If an existing pass manager exists, then merge the given pass manager
@@ -744,25 +844,50 @@ void OpToOpPassAdaptor::runOnOperation(bool verifyPasses) {
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor synchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
auto am = getAnalysisManager();
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
auto *instrumentor = am.getPassInstrumentor();
+
+ unsigned processedOps = 0;
for (auto ®ion : getOperation()->getRegions()) {
for (auto &block : region) {
for (auto &op : block) {
auto *mgr = findPassManagerFor(mgrs, op.getName(), *op.getContext());
- if (!mgr)
+ if (!mgr) {
+ LDBG(2) << "Skipping operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "': no suitable pass manager found";
continue;
+ }
// Run the held pipeline over the current operation.
+ LDBG(2) << "Processing operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' with pass manager '" << mgr->getOpAnchorName() << "'";
+
unsigned initGeneration = mgr->impl->initializationGeneration;
if (failed(runPipeline(*mgr, &op, am.nest(&op), verifyPasses,
- initGeneration, instrumentor, &parentInfo)))
+ initGeneration, instrumentor, &parentInfo))) {
+ LDBG(2) << "Pipeline failed for operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions()) << "'";
signalPassFailure();
+ } else {
+ processedOps++;
+ }
}
}
}
+
+ LDBG() << "Completed synchronous pass adaptor run, processed " << processedOps
+ << " operations";
}
/// Utility functor that checks if the two ranges of pass managers have a size
@@ -776,13 +901,23 @@ static bool hasSizeMismatch(ArrayRef<OpPassManager> lhs,
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor asynchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
AnalysisManager am = getAnalysisManager();
MLIRContext *context = &getContext();
// Create the async executors if they haven't been created, or if the main
// pipeline has changed.
- if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs))
+ if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs)) {
+ LDBG(2) << "Creating " << context->getThreadPool().getMaxConcurrency()
+ << " async executors";
asyncExecutors.assign(context->getThreadPool().getMaxConcurrency(), mgrs);
+ }
// This struct represents the information for a single operation to be
// scheduled on a pass manager.
@@ -803,21 +938,36 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
// operation, as well as providing a queue of operations to execute over.
std::vector<OpPMInfo> opInfos;
DenseMap<OperationName, std::optional<unsigned>> knownOpPMIdx;
+
+ LDBG(2) << "Collecting operations for async execution";
for (auto ®ion : getOperation()->getRegions()) {
for (Operation &op : region.getOps()) {
// Get the pass manager index for this operation type.
auto pmIdxIt = knownOpPMIdx.try_emplace(op.getName(), std::nullopt);
if (pmIdxIt.second) {
- if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context))
+ if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context)) {
pmIdxIt.first->second = std::distance(mgrs.begin(), mgr);
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' will use pass manager '" << mgr->getOpAnchorName()
+ << "'";
+ }
}
// If this operation can be scheduled, add it to the list.
- if (pmIdxIt.first->second)
+ if (pmIdxIt.first->second) {
opInfos.emplace_back(*pmIdxIt.first->second, &op, am.nest(&op));
+ } else {
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' skipped: no suitable pass manager";
+ }
}
}
+ LDBG(2) << "Collected " << opInfos.size()
+ << " operations for async execution";
+
// Get the current thread for this adaptor.
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
@@ -872,23 +1022,35 @@ void PassManager::enableVerifier(bool enabled) { verifyPasses = enabled; }
/// Run the passes within this manager on the provided operation.
LogicalResult PassManager::run(Operation *op) {
+ LDBG() << "Starting PassManager run on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << size() << " passes, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
MLIRContext *context = getContext();
std::optional<OperationName> anchorOp = getOpName(*context);
- if (anchorOp && anchorOp != op->getName())
+ if (anchorOp && anchorOp != op->getName()) {
return emitError(op->getLoc())
<< "can't run '" << getOpAnchorName() << "' pass manager on '"
<< op->getName() << "' op";
+ }
// Register all dialects for the current pipeline.
+ LDBG(2) << "Registering dependent dialects for pipeline";
DialectRegistry dependentDialects;
getDependentDialects(dependentDialects);
context->appendDialectRegistry(dependentDialects);
- for (StringRef name : dependentDialects.getDialectNames())
+ for (StringRef name : dependentDialects.getDialectNames()) {
+ LDBG(2) << "Loading dialect: " << name;
context->getOrLoadDialect(name);
+ }
// Before running, make sure to finalize the pipeline pass list.
- if (failed(getImpl().finalizePassList(context)))
+ if (failed(getImpl().finalizePassList(context))) {
+ LDBG(2) << "Pass list finalization failed";
return failure();
+ }
// Notify the context that we start running a pipeline for bookkeeping.
context->enterMultiThreadedExecution();
@@ -898,17 +1060,27 @@ LogicalResult PassManager::run(Operation *op) {
llvm::hash_code pipelineKey = hash();
if (newInitKey != initializationKey ||
...
[truncated]
|
|
@llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) ChangesPatch is 21.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156205.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index dce706e196bde..bbb4a02066951 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -76,6 +76,13 @@ namespace llvm {
#define __LLVM_FILE_NAME__ ::llvm::impl::getShortFileName(__FILE__)
#endif
+#define LDBG_OS(name, LEVEL) \
+ ::llvm::impl::raw_ldbg_ostream name { \
+ ::llvm::impl::computePrefix(DEBUG_TYPE, __LLVM_FILE_NAME__, __LINE__, \
+ LEVEL), \
+ llvm::dbgs() \
+ }
+
#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, \
LINE) \
for (bool _c = \
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 7094c8e279f2d..9b2491c185868 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -21,11 +21,14 @@
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/DebugLog.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/Threading.h"
#include <optional>
+#define DEBUG_TYPE "pass-manager"
+
using namespace mlir;
using namespace mlir::detail;
@@ -242,6 +245,7 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
};
// Walk the pass list and merge adjacent adaptors.
+ LDBG(3) << "Merging adjacent adaptors in pass list";
OpToOpPassAdaptor *lastAdaptor = nullptr;
for (auto &pass : passes) {
// Check to see if this pass is an adaptor.
@@ -249,18 +253,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// If it is the first adaptor in a possible chain, remember it and
// continue.
if (!lastAdaptor) {
+ LDBG(3) << "Found first adaptor in chain";
lastAdaptor = currentAdaptor;
continue;
}
// Otherwise, try to merge into the existing adaptor and delete the
// current one. If merging fails, just remember this as the last adaptor.
- if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor)))
+ LDBG(3) << "Attempting to merge adaptor with "
+ << currentAdaptor->getPassManagers().size()
+ << " managers into previous adaptor";
+ if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor))) {
+ LDBG(3) << "Successfully merged adaptors, removing current one";
pass.reset();
- else
+ } else {
+ LDBG(3) << "Failed to merge adaptors, keeping current as last";
lastAdaptor = currentAdaptor;
+ }
} else if (lastAdaptor) {
// If this pass isn't an adaptor, finalize it and forget the last adaptor.
+ LDBG(3) << "Finalizing adaptor chain before non-adaptor pass";
if (failed(finalizeAdaptor(lastAdaptor)))
return failure();
lastAdaptor = nullptr;
@@ -273,15 +285,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// Now that the adaptors have been merged, erase any empty slots corresponding
// to the merged adaptors that were nulled-out in the loop above.
+ size_t beforeErase = passes.size();
llvm::erase_if(passes, std::logical_not<std::unique_ptr<Pass>>());
+ if (beforeErase != passes.size()) {
+ LDBG(3) << "Removed " << (beforeErase - passes.size())
+ << " merged adaptor slots from pass list";
+ }
// If this is a op-agnostic pass manager, there is nothing left to do.
std::optional<OperationName> rawOpName = getOpName(*ctx);
- if (!rawOpName)
+ if (!rawOpName) {
+ LDBG(3)
+ << "Op-agnostic pass manager, skipping operation-specific verification";
return success();
+ }
// Otherwise, verify that all of the passes are valid for the current
// operation anchor.
+ LDBG(3) << "Verifying " << passes.size() << " passes for operation '"
+ << getOpAnchorName() << "'";
+
std::optional<RegisteredOperationName> opName =
rawOpName->getRegisteredInfo();
for (std::unique_ptr<Pass> &pass : passes) {
@@ -292,6 +315,8 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
<< "'!";
}
}
+
+ LDBG(3) << "Pass list finalization completed successfully";
return success();
}
@@ -456,23 +481,44 @@ OpPassManager::Nesting OpPassManager::getNesting() { return impl->nesting; }
LogicalResult OpPassManager::initialize(MLIRContext *context,
unsigned newInitGeneration) {
- if (impl->initializationGeneration == newInitGeneration)
+
+ if (impl->initializationGeneration == newInitGeneration) {
+ LDBG(2) << "Pass manager already initialized "
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
return success();
+ }
+
+ LDBG(2) << "Initializing pass manager '" << getOpAnchorName()
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
impl->initializationGeneration = newInitGeneration;
+
for (Pass &pass : getPasses()) {
// If this pass isn't an adaptor, directly initialize it.
auto *adaptor = dyn_cast<OpToOpPassAdaptor>(&pass);
if (!adaptor) {
- if (failed(pass.initialize(context)))
+ LDBG(2) << "Initializing pass '" << pass.getName() << "'";
+ if (failed(pass.initialize(context))) {
+ LDBG(2) << "Failed to initialize pass '" << pass.getName() << "'";
return failure();
+ }
continue;
}
// Otherwise, initialize each of the adaptors pass managers.
+ LDBG(3) << "Initializing adaptor pass with "
+ << adaptor->getPassManagers().size() << " nested managers";
for (OpPassManager &adaptorPM : adaptor->getPassManagers())
- if (failed(adaptorPM.initialize(context, newInitGeneration)))
+ if (failed(adaptorPM.initialize(context, newInitGeneration))) {
+ LDBG(2) << "Failed to initialize nested pass manager";
return failure();
+ }
}
+
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Pass manager initialization completed successfully: ";
+ printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
return success();
}
@@ -499,16 +545,23 @@ llvm::hash_code OpPassManager::hash() {
LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration) {
+ LDBG() << "Running pass '" << pass->getName() << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' at "
+ << op->getLoc();
+
std::optional<RegisteredOperationName> opInfo = op->getRegisteredInfo();
- if (!opInfo)
+ if (!opInfo) {
return op->emitOpError()
<< "trying to schedule a pass on an unregistered operation";
- if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>())
+ }
+ if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>()) {
return op->emitOpError() << "trying to schedule a pass on an operation not "
"marked as 'IsolatedFromAbove'";
- if (!pass->canScheduleOn(*op->getName().getRegisteredInfo()))
+ }
+ if (!pass->canScheduleOn(*op->getName().getRegisteredInfo())) {
return op->emitOpError()
<< "trying to schedule a pass on an unsupported operation";
+ }
// Initialize the pass state with a callback for the pass to dynamically
// execute a pipeline on the currently visited operation.
@@ -526,8 +579,10 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
pipeline.getImpl().canScheduleOn(*op->getContext(), root->getName()));
// Before running, finalize the passes held by the pipeline.
- if (failed(pipeline.getImpl().finalizePassList(root->getContext())))
+ if (failed(pipeline.getImpl().finalizePassList(root->getContext()))) {
+ LDBG() << "Failed to finalize pass list for pipeline";
return failure();
+ }
// Initialize the user provided pipeline and execute the pipeline.
if (failed(pipeline.initialize(root->getContext(), parentInitGeneration)))
@@ -599,6 +654,11 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
OpPassManager &pm, Operation *op, AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration, PassInstrumentor *instrumentor,
const PassInstrumentation::PipelineParentInfo *parentInfo) {
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Running pipeline on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << pm.size() << " passes, verifyPasses=" << verifyPasses;
+ pm.printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
assert((!instrumentor || parentInfo) &&
"expected parent info if instrumentor is provided");
auto scopeExit = llvm::make_scope_exit([&] {
@@ -615,9 +675,14 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
*parentInfo);
}
- for (Pass &pass : pm.getPasses())
- if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration)))
+ for (Pass &pass : pm.getPasses()) {
+ if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration))) {
+ LDBG() << "Pipeline failed for pass '" << pass.getName()
+ << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "'";
return failure();
+ }
+ }
if (instrumentor) {
instrumentor->runAfterPipeline(pm.getOpName(*op->getContext()),
@@ -630,9 +695,19 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
/// does not exist.
static OpPassManager *
findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
+ LDBG(3) << "Looking for pass manager with anchor name '" << name << "' among "
+ << mgrs.size() << " managers";
+
auto *it = llvm::find_if(
mgrs, [&](OpPassManager &mgr) { return mgr.getOpAnchorName() == name; });
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(2) << "No pass manager found with anchor name '" << name << "'";
+ return nullptr;
+ }
+
+ LDBG(2) << "Found pass manager with anchor name '" << name << "'";
+ return &*it;
}
/// Find an operation pass manager that can operate on an operation of the given
@@ -640,10 +715,22 @@ findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
static OpPassManager *findPassManagerFor(MutableArrayRef<OpPassManager> mgrs,
OperationName name,
MLIRContext &context) {
+ LDBG(4) << "Looking for pass manager that can handle operation '" << name
+ << "' among " << mgrs.size() << " managers";
+
auto *it = llvm::find_if(mgrs, [&](OpPassManager &mgr) {
return mgr.getImpl().canScheduleOn(context, name);
});
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(4) << "No pass manager found that can handle operation '" << name
+ << "'";
+ return nullptr;
+ }
+
+ LDBG(4) << "Found pass manager '" << it->getOpAnchorName()
+ << "' that can handle operation '" << name << "'";
+ return &*it;
}
OpToOpPassAdaptor::OpToOpPassAdaptor(OpPassManager &&mgr) {
@@ -657,6 +744,9 @@ void OpToOpPassAdaptor::getDependentDialects(DialectRegistry &dialects) const {
LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
OpToOpPassAdaptor &rhs) {
+ LDBG(3) << "Attempting to merge pass adaptor with " << mgrs.size()
+ << " managers into rhs with " << rhs.mgrs.size() << " managers";
+
// Functor used to check if a pass manager is generic, i.e. op-agnostic.
auto isGenericPM = [&](OpPassManager &pm) { return !pm.getOpName(); };
@@ -682,14 +772,24 @@ LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
//
// Check the current adaptor.
auto *lhsGenericPMIt = llvm::find_if(mgrs, isGenericPM);
- if (lhsGenericPMIt != mgrs.end() &&
- hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs))
- return failure();
+ if (lhsGenericPMIt != mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on LHS, checking for conflicts";
+ if (hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs)) {
+ LDBG(4)
+ << "Merge failed: LHS generic pass manager has conflicts with RHS";
+ return failure();
+ }
+ }
// Check the rhs adaptor.
auto *rhsGenericPMIt = llvm::find_if(rhs.mgrs, isGenericPM);
- if (rhsGenericPMIt != rhs.mgrs.end() &&
- hasScheduleConflictWith(*rhsGenericPMIt, mgrs))
- return failure();
+ if (rhsGenericPMIt != rhs.mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on RHS, checking for conflicts";
+ if (hasScheduleConflictWith(*rhsGenericPMIt, mgrs)) {
+ LDBG(4)
+ << "Merge failed: RHS generic pass manager has conflicts with LHS";
+ return failure();
+ }
+ }
for (auto &pm : mgrs) {
// If an existing pass manager exists, then merge the given pass manager
@@ -744,25 +844,50 @@ void OpToOpPassAdaptor::runOnOperation(bool verifyPasses) {
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor synchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
auto am = getAnalysisManager();
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
auto *instrumentor = am.getPassInstrumentor();
+
+ unsigned processedOps = 0;
for (auto ®ion : getOperation()->getRegions()) {
for (auto &block : region) {
for (auto &op : block) {
auto *mgr = findPassManagerFor(mgrs, op.getName(), *op.getContext());
- if (!mgr)
+ if (!mgr) {
+ LDBG(2) << "Skipping operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "': no suitable pass manager found";
continue;
+ }
// Run the held pipeline over the current operation.
+ LDBG(2) << "Processing operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' with pass manager '" << mgr->getOpAnchorName() << "'";
+
unsigned initGeneration = mgr->impl->initializationGeneration;
if (failed(runPipeline(*mgr, &op, am.nest(&op), verifyPasses,
- initGeneration, instrumentor, &parentInfo)))
+ initGeneration, instrumentor, &parentInfo))) {
+ LDBG(2) << "Pipeline failed for operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions()) << "'";
signalPassFailure();
+ } else {
+ processedOps++;
+ }
}
}
}
+
+ LDBG() << "Completed synchronous pass adaptor run, processed " << processedOps
+ << " operations";
}
/// Utility functor that checks if the two ranges of pass managers have a size
@@ -776,13 +901,23 @@ static bool hasSizeMismatch(ArrayRef<OpPassManager> lhs,
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor asynchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
AnalysisManager am = getAnalysisManager();
MLIRContext *context = &getContext();
// Create the async executors if they haven't been created, or if the main
// pipeline has changed.
- if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs))
+ if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs)) {
+ LDBG(2) << "Creating " << context->getThreadPool().getMaxConcurrency()
+ << " async executors";
asyncExecutors.assign(context->getThreadPool().getMaxConcurrency(), mgrs);
+ }
// This struct represents the information for a single operation to be
// scheduled on a pass manager.
@@ -803,21 +938,36 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
// operation, as well as providing a queue of operations to execute over.
std::vector<OpPMInfo> opInfos;
DenseMap<OperationName, std::optional<unsigned>> knownOpPMIdx;
+
+ LDBG(2) << "Collecting operations for async execution";
for (auto ®ion : getOperation()->getRegions()) {
for (Operation &op : region.getOps()) {
// Get the pass manager index for this operation type.
auto pmIdxIt = knownOpPMIdx.try_emplace(op.getName(), std::nullopt);
if (pmIdxIt.second) {
- if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context))
+ if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context)) {
pmIdxIt.first->second = std::distance(mgrs.begin(), mgr);
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' will use pass manager '" << mgr->getOpAnchorName()
+ << "'";
+ }
}
// If this operation can be scheduled, add it to the list.
- if (pmIdxIt.first->second)
+ if (pmIdxIt.first->second) {
opInfos.emplace_back(*pmIdxIt.first->second, &op, am.nest(&op));
+ } else {
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' skipped: no suitable pass manager";
+ }
}
}
+ LDBG(2) << "Collected " << opInfos.size()
+ << " operations for async execution";
+
// Get the current thread for this adaptor.
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
@@ -872,23 +1022,35 @@ void PassManager::enableVerifier(bool enabled) { verifyPasses = enabled; }
/// Run the passes within this manager on the provided operation.
LogicalResult PassManager::run(Operation *op) {
+ LDBG() << "Starting PassManager run on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << size() << " passes, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
MLIRContext *context = getContext();
std::optional<OperationName> anchorOp = getOpName(*context);
- if (anchorOp && anchorOp != op->getName())
+ if (anchorOp && anchorOp != op->getName()) {
return emitError(op->getLoc())
<< "can't run '" << getOpAnchorName() << "' pass manager on '"
<< op->getName() << "' op";
+ }
// Register all dialects for the current pipeline.
+ LDBG(2) << "Registering dependent dialects for pipeline";
DialectRegistry dependentDialects;
getDependentDialects(dependentDialects);
context->appendDialectRegistry(dependentDialects);
- for (StringRef name : dependentDialects.getDialectNames())
+ for (StringRef name : dependentDialects.getDialectNames()) {
+ LDBG(2) << "Loading dialect: " << name;
context->getOrLoadDialect(name);
+ }
// Before running, make sure to finalize the pipeline pass list.
- if (failed(getImpl().finalizePassList(context)))
+ if (failed(getImpl().finalizePassList(context))) {
+ LDBG(2) << "Pass list finalization failed";
return failure();
+ }
// Notify the context that we start running a pipeline for bookkeeping.
context->enterMultiThreadedExecution();
@@ -898,17 +1060,27 @@ LogicalResult PassManager::run(Operation *op) {
llvm::hash_code pipelineKey = hash();
if (newInitKey != initializationKey ||
...
[truncated]
|
|
@llvm/pr-subscribers-mlir-core Author: Mehdi Amini (joker-eph) ChangesPatch is 21.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156205.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index dce706e196bde..bbb4a02066951 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -76,6 +76,13 @@ namespace llvm {
#define __LLVM_FILE_NAME__ ::llvm::impl::getShortFileName(__FILE__)
#endif
+#define LDBG_OS(name, LEVEL) \
+ ::llvm::impl::raw_ldbg_ostream name { \
+ ::llvm::impl::computePrefix(DEBUG_TYPE, __LLVM_FILE_NAME__, __LINE__, \
+ LEVEL), \
+ llvm::dbgs() \
+ }
+
#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, \
LINE) \
for (bool _c = \
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 7094c8e279f2d..9b2491c185868 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -21,11 +21,14 @@
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/DebugLog.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/Threading.h"
#include <optional>
+#define DEBUG_TYPE "pass-manager"
+
using namespace mlir;
using namespace mlir::detail;
@@ -242,6 +245,7 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
};
// Walk the pass list and merge adjacent adaptors.
+ LDBG(3) << "Merging adjacent adaptors in pass list";
OpToOpPassAdaptor *lastAdaptor = nullptr;
for (auto &pass : passes) {
// Check to see if this pass is an adaptor.
@@ -249,18 +253,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// If it is the first adaptor in a possible chain, remember it and
// continue.
if (!lastAdaptor) {
+ LDBG(3) << "Found first adaptor in chain";
lastAdaptor = currentAdaptor;
continue;
}
// Otherwise, try to merge into the existing adaptor and delete the
// current one. If merging fails, just remember this as the last adaptor.
- if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor)))
+ LDBG(3) << "Attempting to merge adaptor with "
+ << currentAdaptor->getPassManagers().size()
+ << " managers into previous adaptor";
+ if (succeeded(currentAdaptor->tryMergeInto(ctx, *lastAdaptor))) {
+ LDBG(3) << "Successfully merged adaptors, removing current one";
pass.reset();
- else
+ } else {
+ LDBG(3) << "Failed to merge adaptors, keeping current as last";
lastAdaptor = currentAdaptor;
+ }
} else if (lastAdaptor) {
// If this pass isn't an adaptor, finalize it and forget the last adaptor.
+ LDBG(3) << "Finalizing adaptor chain before non-adaptor pass";
if (failed(finalizeAdaptor(lastAdaptor)))
return failure();
lastAdaptor = nullptr;
@@ -273,15 +285,26 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
// Now that the adaptors have been merged, erase any empty slots corresponding
// to the merged adaptors that were nulled-out in the loop above.
+ size_t beforeErase = passes.size();
llvm::erase_if(passes, std::logical_not<std::unique_ptr<Pass>>());
+ if (beforeErase != passes.size()) {
+ LDBG(3) << "Removed " << (beforeErase - passes.size())
+ << " merged adaptor slots from pass list";
+ }
// If this is a op-agnostic pass manager, there is nothing left to do.
std::optional<OperationName> rawOpName = getOpName(*ctx);
- if (!rawOpName)
+ if (!rawOpName) {
+ LDBG(3)
+ << "Op-agnostic pass manager, skipping operation-specific verification";
return success();
+ }
// Otherwise, verify that all of the passes are valid for the current
// operation anchor.
+ LDBG(3) << "Verifying " << passes.size() << " passes for operation '"
+ << getOpAnchorName() << "'";
+
std::optional<RegisteredOperationName> opName =
rawOpName->getRegisteredInfo();
for (std::unique_ptr<Pass> &pass : passes) {
@@ -292,6 +315,8 @@ LogicalResult OpPassManagerImpl::finalizePassList(MLIRContext *ctx) {
<< "'!";
}
}
+
+ LDBG(3) << "Pass list finalization completed successfully";
return success();
}
@@ -456,23 +481,44 @@ OpPassManager::Nesting OpPassManager::getNesting() { return impl->nesting; }
LogicalResult OpPassManager::initialize(MLIRContext *context,
unsigned newInitGeneration) {
- if (impl->initializationGeneration == newInitGeneration)
+
+ if (impl->initializationGeneration == newInitGeneration) {
+ LDBG(2) << "Pass manager already initialized "
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
return success();
+ }
+
+ LDBG(2) << "Initializing pass manager '" << getOpAnchorName()
+ << "' (generation " << newInitGeneration << ") with " << size()
+ << " passes";
impl->initializationGeneration = newInitGeneration;
+
for (Pass &pass : getPasses()) {
// If this pass isn't an adaptor, directly initialize it.
auto *adaptor = dyn_cast<OpToOpPassAdaptor>(&pass);
if (!adaptor) {
- if (failed(pass.initialize(context)))
+ LDBG(2) << "Initializing pass '" << pass.getName() << "'";
+ if (failed(pass.initialize(context))) {
+ LDBG(2) << "Failed to initialize pass '" << pass.getName() << "'";
return failure();
+ }
continue;
}
// Otherwise, initialize each of the adaptors pass managers.
+ LDBG(3) << "Initializing adaptor pass with "
+ << adaptor->getPassManagers().size() << " nested managers";
for (OpPassManager &adaptorPM : adaptor->getPassManagers())
- if (failed(adaptorPM.initialize(context, newInitGeneration)))
+ if (failed(adaptorPM.initialize(context, newInitGeneration))) {
+ LDBG(2) << "Failed to initialize nested pass manager";
return failure();
+ }
}
+
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Pass manager initialization completed successfully: ";
+ printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
return success();
}
@@ -499,16 +545,23 @@ llvm::hash_code OpPassManager::hash() {
LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration) {
+ LDBG() << "Running pass '" << pass->getName() << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' at "
+ << op->getLoc();
+
std::optional<RegisteredOperationName> opInfo = op->getRegisteredInfo();
- if (!opInfo)
+ if (!opInfo) {
return op->emitOpError()
<< "trying to schedule a pass on an unregistered operation";
- if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>())
+ }
+ if (!opInfo->hasTrait<OpTrait::IsIsolatedFromAbove>()) {
return op->emitOpError() << "trying to schedule a pass on an operation not "
"marked as 'IsolatedFromAbove'";
- if (!pass->canScheduleOn(*op->getName().getRegisteredInfo()))
+ }
+ if (!pass->canScheduleOn(*op->getName().getRegisteredInfo())) {
return op->emitOpError()
<< "trying to schedule a pass on an unsupported operation";
+ }
// Initialize the pass state with a callback for the pass to dynamically
// execute a pipeline on the currently visited operation.
@@ -526,8 +579,10 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
pipeline.getImpl().canScheduleOn(*op->getContext(), root->getName()));
// Before running, finalize the passes held by the pipeline.
- if (failed(pipeline.getImpl().finalizePassList(root->getContext())))
+ if (failed(pipeline.getImpl().finalizePassList(root->getContext()))) {
+ LDBG() << "Failed to finalize pass list for pipeline";
return failure();
+ }
// Initialize the user provided pipeline and execute the pipeline.
if (failed(pipeline.initialize(root->getContext(), parentInitGeneration)))
@@ -599,6 +654,11 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
OpPassManager &pm, Operation *op, AnalysisManager am, bool verifyPasses,
unsigned parentInitGeneration, PassInstrumentor *instrumentor,
const PassInstrumentation::PipelineParentInfo *parentInfo) {
+ LLVM_DEBUG(LDBG_OS(os, 1);
+ os << "Running pipeline on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << pm.size() << " passes, verifyPasses=" << verifyPasses;
+ pm.printAsTextualPipeline(os, /*pretty=*/false); os << "\n");
assert((!instrumentor || parentInfo) &&
"expected parent info if instrumentor is provided");
auto scopeExit = llvm::make_scope_exit([&] {
@@ -615,9 +675,14 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
*parentInfo);
}
- for (Pass &pass : pm.getPasses())
- if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration)))
+ for (Pass &pass : pm.getPasses()) {
+ if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration))) {
+ LDBG() << "Pipeline failed for pass '" << pass.getName()
+ << "' on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "'";
return failure();
+ }
+ }
if (instrumentor) {
instrumentor->runAfterPipeline(pm.getOpName(*op->getContext()),
@@ -630,9 +695,19 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
/// does not exist.
static OpPassManager *
findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
+ LDBG(3) << "Looking for pass manager with anchor name '" << name << "' among "
+ << mgrs.size() << " managers";
+
auto *it = llvm::find_if(
mgrs, [&](OpPassManager &mgr) { return mgr.getOpAnchorName() == name; });
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(2) << "No pass manager found with anchor name '" << name << "'";
+ return nullptr;
+ }
+
+ LDBG(2) << "Found pass manager with anchor name '" << name << "'";
+ return &*it;
}
/// Find an operation pass manager that can operate on an operation of the given
@@ -640,10 +715,22 @@ findPassManagerWithAnchor(MutableArrayRef<OpPassManager> mgrs, StringRef name) {
static OpPassManager *findPassManagerFor(MutableArrayRef<OpPassManager> mgrs,
OperationName name,
MLIRContext &context) {
+ LDBG(4) << "Looking for pass manager that can handle operation '" << name
+ << "' among " << mgrs.size() << " managers";
+
auto *it = llvm::find_if(mgrs, [&](OpPassManager &mgr) {
return mgr.getImpl().canScheduleOn(context, name);
});
- return it == mgrs.end() ? nullptr : &*it;
+
+ if (it == mgrs.end()) {
+ LDBG(4) << "No pass manager found that can handle operation '" << name
+ << "'";
+ return nullptr;
+ }
+
+ LDBG(4) << "Found pass manager '" << it->getOpAnchorName()
+ << "' that can handle operation '" << name << "'";
+ return &*it;
}
OpToOpPassAdaptor::OpToOpPassAdaptor(OpPassManager &&mgr) {
@@ -657,6 +744,9 @@ void OpToOpPassAdaptor::getDependentDialects(DialectRegistry &dialects) const {
LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
OpToOpPassAdaptor &rhs) {
+ LDBG(3) << "Attempting to merge pass adaptor with " << mgrs.size()
+ << " managers into rhs with " << rhs.mgrs.size() << " managers";
+
// Functor used to check if a pass manager is generic, i.e. op-agnostic.
auto isGenericPM = [&](OpPassManager &pm) { return !pm.getOpName(); };
@@ -682,14 +772,24 @@ LogicalResult OpToOpPassAdaptor::tryMergeInto(MLIRContext *ctx,
//
// Check the current adaptor.
auto *lhsGenericPMIt = llvm::find_if(mgrs, isGenericPM);
- if (lhsGenericPMIt != mgrs.end() &&
- hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs))
- return failure();
+ if (lhsGenericPMIt != mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on LHS, checking for conflicts";
+ if (hasScheduleConflictWith(*lhsGenericPMIt, rhs.mgrs)) {
+ LDBG(4)
+ << "Merge failed: LHS generic pass manager has conflicts with RHS";
+ return failure();
+ }
+ }
// Check the rhs adaptor.
auto *rhsGenericPMIt = llvm::find_if(rhs.mgrs, isGenericPM);
- if (rhsGenericPMIt != rhs.mgrs.end() &&
- hasScheduleConflictWith(*rhsGenericPMIt, mgrs))
- return failure();
+ if (rhsGenericPMIt != rhs.mgrs.end()) {
+ LDBG(4) << "Found generic pass manager on RHS, checking for conflicts";
+ if (hasScheduleConflictWith(*rhsGenericPMIt, mgrs)) {
+ LDBG(4)
+ << "Merge failed: RHS generic pass manager has conflicts with LHS";
+ return failure();
+ }
+ }
for (auto &pm : mgrs) {
// If an existing pass manager exists, then merge the given pass manager
@@ -744,25 +844,50 @@ void OpToOpPassAdaptor::runOnOperation(bool verifyPasses) {
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor synchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
auto am = getAnalysisManager();
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
auto *instrumentor = am.getPassInstrumentor();
+
+ unsigned processedOps = 0;
for (auto ®ion : getOperation()->getRegions()) {
for (auto &block : region) {
for (auto &op : block) {
auto *mgr = findPassManagerFor(mgrs, op.getName(), *op.getContext());
- if (!mgr)
+ if (!mgr) {
+ LDBG(2) << "Skipping operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "': no suitable pass manager found";
continue;
+ }
// Run the held pipeline over the current operation.
+ LDBG(2) << "Processing operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' with pass manager '" << mgr->getOpAnchorName() << "'";
+
unsigned initGeneration = mgr->impl->initializationGeneration;
if (failed(runPipeline(*mgr, &op, am.nest(&op), verifyPasses,
- initGeneration, instrumentor, &parentInfo)))
+ initGeneration, instrumentor, &parentInfo))) {
+ LDBG(2) << "Pipeline failed for operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions()) << "'";
signalPassFailure();
+ } else {
+ processedOps++;
+ }
}
}
}
+
+ LDBG() << "Completed synchronous pass adaptor run, processed " << processedOps
+ << " operations";
}
/// Utility functor that checks if the two ranges of pass managers have a size
@@ -776,13 +901,23 @@ static bool hasSizeMismatch(ArrayRef<OpPassManager> lhs,
/// Run this pass adaptor synchronously.
void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
+ LDBG() << "Running pass adaptor asynchronously on operation '"
+ << OpWithFlags(getOperation(), OpPrintingFlags().skipRegions())
+ << "' with " << mgrs.size()
+ << " pass managers, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
AnalysisManager am = getAnalysisManager();
MLIRContext *context = &getContext();
// Create the async executors if they haven't been created, or if the main
// pipeline has changed.
- if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs))
+ if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs)) {
+ LDBG(2) << "Creating " << context->getThreadPool().getMaxConcurrency()
+ << " async executors";
asyncExecutors.assign(context->getThreadPool().getMaxConcurrency(), mgrs);
+ }
// This struct represents the information for a single operation to be
// scheduled on a pass manager.
@@ -803,21 +938,36 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
// operation, as well as providing a queue of operations to execute over.
std::vector<OpPMInfo> opInfos;
DenseMap<OperationName, std::optional<unsigned>> knownOpPMIdx;
+
+ LDBG(2) << "Collecting operations for async execution";
for (auto ®ion : getOperation()->getRegions()) {
for (Operation &op : region.getOps()) {
// Get the pass manager index for this operation type.
auto pmIdxIt = knownOpPMIdx.try_emplace(op.getName(), std::nullopt);
if (pmIdxIt.second) {
- if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context))
+ if (auto *mgr = findPassManagerFor(mgrs, op.getName(), *context)) {
pmIdxIt.first->second = std::distance(mgrs.begin(), mgr);
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' will use pass manager '" << mgr->getOpAnchorName()
+ << "'";
+ }
}
// If this operation can be scheduled, add it to the list.
- if (pmIdxIt.first->second)
+ if (pmIdxIt.first->second) {
opInfos.emplace_back(*pmIdxIt.first->second, &op, am.nest(&op));
+ } else {
+ LDBG(2) << "Operation '"
+ << OpWithFlags(&op, OpPrintingFlags().skipRegions())
+ << "' skipped: no suitable pass manager";
+ }
}
}
+ LDBG(2) << "Collected " << opInfos.size()
+ << " operations for async execution";
+
// Get the current thread for this adaptor.
PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
this};
@@ -872,23 +1022,35 @@ void PassManager::enableVerifier(bool enabled) { verifyPasses = enabled; }
/// Run the passes within this manager on the provided operation.
LogicalResult PassManager::run(Operation *op) {
+ LDBG() << "Starting PassManager run on operation '"
+ << OpWithFlags(op, OpPrintingFlags().skipRegions()) << "' with "
+ << size() << " passes, verifyPasses=" << verifyPasses;
+ LLVM_DEBUG(LDBG_OS(os, 1); printAsTextualPipeline(os, /*pretty=*/false);
+ os << "\n");
+
MLIRContext *context = getContext();
std::optional<OperationName> anchorOp = getOpName(*context);
- if (anchorOp && anchorOp != op->getName())
+ if (anchorOp && anchorOp != op->getName()) {
return emitError(op->getLoc())
<< "can't run '" << getOpAnchorName() << "' pass manager on '"
<< op->getName() << "' op";
+ }
// Register all dialects for the current pipeline.
+ LDBG(2) << "Registering dependent dialects for pipeline";
DialectRegistry dependentDialects;
getDependentDialects(dependentDialects);
context->appendDialectRegistry(dependentDialects);
- for (StringRef name : dependentDialects.getDialectNames())
+ for (StringRef name : dependentDialects.getDialectNames()) {
+ LDBG(2) << "Loading dialect: " << name;
context->getOrLoadDialect(name);
+ }
// Before running, make sure to finalize the pipeline pass list.
- if (failed(getImpl().finalizePassList(context)))
+ if (failed(getImpl().finalizePassList(context))) {
+ LDBG(2) << "Pass list finalization failed";
return failure();
+ }
// Notify the context that we start running a pipeline for bookkeeping.
context->enterMultiThreadedExecution();
@@ -898,17 +1060,27 @@ LogicalResult PassManager::run(Operation *op) {
llvm::hash_code pipelineKey = hash();
if (newInitKey != initializationKey ||
...
[truncated]
|
joker-eph
force-pushed
the
ldbg_passmanager
branch
from
c307906 to
f36ce51
Compare
jpienaar
requested changes
Sep 3, 2025
llvm/include/llvm/Support/DebugLog.h
Outdated
| @@ -76,6 +76,13 @@ namespace llvm { | |||
| #define __LLVM_FILE_NAME__ ::llvm::impl::getShortFileName(__FILE__) | |||
| #endif | |||
|
|
|||
| #define LDBG_OS(name, LEVEL) \ | |||
There was a problem hiding this comment.
The name is a bit confusing as it could also have been seen as debug type one. Could it be done so that it is inline? (and given inheritance, they could just assign to variable). Then the below could become
printAsTextualPipeline(LDBG_OS(1), /pretty=/false));
and
raw_ostream& os = LDBG_OS(type, level); ?
joker-eph
force-pushed
the
ldbg_passmanager
branch
from
f36ce51 to
e4e1a0e
Compare
joker-eph
requested review from
banach-space,
dcaballe,
hanhanW and
nicolasvasilache
as code owners
joker-eph
force-pushed
the
ldbg_passmanager
branch
2 times, most recently
from
0185b4a to
b2c37c6
Compare
banach-space
reviewed
Sep 5, 2025
joker-eph
force-pushed
the
ldbg_passmanager
branch
from
b2c37c6 to
93fae3d
Compare
|
This is obviously great, thanks for implementing it! In general looks good - my comments are minor. It would be good to get an opinion from somebody from a different sub-project too (given that this is common code). Btw, a nit, please avoid force-pushing: 
Thanks! |
FYI: this code is exclusively used in MLIR right now, I've been tweaking this with Jacques for the last month :) I'm actually revamping this patch soon with a different approach!
I'm not sure how to avoid this actually: any chain of work I have locally is unmanageable for me without this (one PR == one commit) |
joker-eph
force-pushed
the
ldbg_passmanager
branch
from
93fae3d to
1570e39
Compare
|
I split the introduction of the LDBG_OS() macro in a new PR: #157194 |
Yes, I've noticed and really appreciate all the improvements. Also happy I finally have cycles to track this a bit more :)
Sure, I know where you are coming from and it makes sense. However, from what I can tell, it's a "chain of work" downstream (as in, I don't see any other patches upstream except for #157194). So, from the "upstream" perspective (i.e., the perspective that reviewers like me have access to), this is an isolated change. By force-pushing, we are loosing the history. Specifically, it is impossible to "connect" comments to code updates and, effectively, to understand how this change evolved (and the rationale underlying the design). All in all, it's a tradeoff - the development is much easier, but that comes at a cost of lost history. I am merely sharing my understanding of our development policies and the rationale behind them - I've been making similar request to other contributors. If you feel that I am misinterpreting our policies, we should probably start a discussion on Discourse and clarify the docs. On a related note, with #156205 we are effectively creating "stacked PRs". So, IIUC, this commit no longer belongs to this PR. Could you either update the summary or use one of the other methods that we use for stacked PRs? Thanks Mehdi! |
We should. I'm pretty sure this was covered during the migration to GitHub pull-request and this was meant as a suggestion and nothing more, I'll do some digging. |
The problem with stacked PR is that you need to know that you'll do a stack before pushing, because the branch needs to be in the llvm repo. When I opened the original PR (before splitting), I unfortunately did from my fork and not from a user branch in the llvm repo, which means that to make this PR part of a stack I have to close the PRs and re-open new ones from branches in the LLVM repo. |
Re-reading our policy:
So yes, you were right, this is just a suggestion and I guess there is no need to bring this to Discourse (unless anyone reading this feels otherwise). That said,
I was also very fond of the Phabricator flow. That's a side point though. I totally understand (and can relate to) that you want to optimise for your development experience. However, this comes at a price to reviewers’ experience in general, especially for folks reading PRs in the future.
That's what I do occasionally and it works fine for me 🤷🏻 Note, however, that in the absence of "user-branches", we recommend a "dependency notes". Let me update the description accordingly. |
What's actually missing for folks who who read a PR in the future? |
joker-eph
force-pushed
the
ldbg_passmanager
branch
from
1570e39 to
50e9462
Compare
joker-eph
added
the
skip-precommit-approval
|
OK this is rebased now. After the split from the infrastructure part, I'm not sure if someone still want to review this? Otherwise I'll just land as-is. |
The history of these changes and how our discussion here shaped it. Phabricator did preserve the intermediate diffs - that isn’t available here anymore. This may feel unnecessary today, but it could be very valuable in the future. From my experience working on Flang and digging through Clang’s history, I learnt the hard way that these review discussions can be a very valuable source of information for our “future” selves and other contributors.
I’ve skimmed through and it mostly looks good. One thing I’d appreciate is some high-level documentation/rationale for what the different logging levels represent. Otherwise, the distinctions could feel a bit arbitrary to future readers. Not a blocker, just a nice-to-have. Thanks! |
What about the "compare" button next to each push? |
TIL :) Thanks for pointing that out! That definitely weakens my argument. Still, as a reviewer, I definitely find it easier to work with multiple commits:
This might be a matter of preference though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.