Skip to content

Commit 3c01ac6

Browse files
joker-ephjoker-eph
authored
[MLIR] Fixup the LDBG() logging in dataflow/deadcodeanalysis (NFC) (#155085)
This is improving the debug output: - avoid printing pointers, print ops without regions in general. - skip extra new-lines in the output - minor other consistency aspects.
1 parent ee8c14b commit 3c01ac6

File tree

3 files changed

+67
-35
lines changed

3 files changed

+67
-35
lines changed

‎mlir/include/mlir/IR/Operation.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,8 @@ inline raw_ostream &operator<<(raw_ostream &os, const Operation &op) {
11061106
/// useful to act as a "stream modifier" to customize printing an operation
11071107
/// with a stream using the operator<< overload, e.g.:
11081108
/// llvm::dbgs() << OpWithFlags(op, OpPrintingFlags().skipRegions());
1109+
/// This always prints the operation with the local scope, to avoid introducing
1110+
/// spurious newlines in the stream.
11091111
class OpWithFlags {
11101112
public:
11111113
OpWithFlags(Operation *op, OpPrintingFlags flags = {})
@@ -1116,11 +1118,11 @@ class OpWithFlags {
11161118
private:
11171119
Operation *op;
11181120
OpPrintingFlags theFlags;
1119-
friend raw_ostream &operator<<(raw_ostream &os, const OpWithFlags &op);
1121+
friend raw_ostream &operator<<(raw_ostream &os, OpWithFlags op);
11201122
};
11211123

1122-
inline raw_ostream &operator<<(raw_ostream &os,
1123-
const OpWithFlags &opWithFlags) {
1124+
inline raw_ostream &operator<<(raw_ostream &os, OpWithFlags opWithFlags) {
1125+
opWithFlags.flags().useLocalScope();
11241126
opWithFlags.op->print(os, opWithFlags.flags());
11251127
return os;
11261128
}

‎mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,17 @@ void Executable::onUpdate(DataFlowSolver *solver) const {
7979
void PredecessorState::print(raw_ostream &os) const {
8080
if (allPredecessorsKnown())
8181
os << "(all) ";
82-
os << "predecessors:\n";
83-
for (Operation *op : getKnownPredecessors())
84-
os << " " << *op << "\n";
82+
os << "predecessors:";
83+
if (getKnownPredecessors().empty())
84+
os << " (none)";
85+
else
86+
os << "\n";
87+
llvm::interleave(
88+
getKnownPredecessors(), os,
89+
[&](Operation *op) {
90+
os << " " << OpWithFlags(op, OpPrintingFlags().skipRegions());
91+
},
92+
"\n");
8593
}
8694

8795
ChangeResult PredecessorState::join(Operation *predecessor) {
@@ -128,15 +136,16 @@ DeadCodeAnalysis::DeadCodeAnalysis(DataFlowSolver &solver)
128136

129137
LogicalResult DeadCodeAnalysis::initialize(Operation *top) {
130138
LDBG() << "Initializing DeadCodeAnalysis for top-level op: "
131-
<< top->getName();
139+
<< OpWithFlags(top, OpPrintingFlags().skipRegions());
132140
// Mark the top-level blocks as executable.
133141
for (Region &region : top->getRegions()) {
134142
if (region.empty())
135143
continue;
136144
auto *state =
137145
getOrCreate<Executable>(getProgramPointBefore(&region.front()));
138146
propagateIfChanged(state, state->setToLive());
139-
LDBG() << "Marked entry block live for region in op: " << top->getName();
147+
LDBG() << "Marked entry block live for region in op: "
148+
<< OpWithFlags(top, OpPrintingFlags().skipRegions());
140149
}
141150

142151
// Mark as overdefined the predecessors of symbol callables with potentially
@@ -151,14 +160,16 @@ void DeadCodeAnalysis::initializeSymbolCallables(Operation *top) {
151160
<< OpWithFlags(top, OpPrintingFlags().skipRegions());
152161
analysisScope = top;
153162
auto walkFn = [&](Operation *symTable, bool allUsesVisible) {
154-
LDBG() << "[init] Processing symbol table op: " << symTable->getName();
163+
LDBG() << "[init] Processing symbol table op: "
164+
<< OpWithFlags(symTable, OpPrintingFlags().skipRegions());
155165
Region &symbolTableRegion = symTable->getRegion(0);
156166
Block *symbolTableBlock = &symbolTableRegion.front();
157167

158168
bool foundSymbolCallable = false;
159169
for (auto callable : symbolTableBlock->getOps<CallableOpInterface>()) {
160170
LDBG() << "[init] Found CallableOpInterface: "
161-
<< callable.getOperation()->getName();
171+
<< OpWithFlags(callable.getOperation(),
172+
OpPrintingFlags().skipRegions());
162173
Region *callableRegion = callable.getCallableRegion();
163174
if (!callableRegion)
164175
continue;
@@ -173,7 +184,8 @@ void DeadCodeAnalysis::initializeSymbolCallables(Operation *top) {
173184
getOrCreate<PredecessorState>(getProgramPointAfter(callable));
174185
propagateIfChanged(state, state->setHasUnknownPredecessors());
175186
LDBG() << "[init] Marked callable as having unknown predecessors: "
176-
<< callable.getOperation()->getName();
187+
<< OpWithFlags(callable.getOperation(),
188+
OpPrintingFlags().skipRegions());
177189
}
178190
foundSymbolCallable = true;
179191
}
@@ -196,7 +208,8 @@ void DeadCodeAnalysis::initializeSymbolCallables(Operation *top) {
196208
propagateIfChanged(state, state->setHasUnknownPredecessors());
197209
LDBG() << "[init] Marked nested callable as "
198210
"having unknown predecessors: "
199-
<< callable.getOperation()->getName();
211+
<< OpWithFlags(callable.getOperation(),
212+
OpPrintingFlags().skipRegions());
200213
});
201214
}
202215

@@ -212,7 +225,7 @@ void DeadCodeAnalysis::initializeSymbolCallables(Operation *top) {
212225
propagateIfChanged(state, state->setHasUnknownPredecessors());
213226
LDBG() << "[init] Found non-call use for symbol, "
214227
"marked as having unknown predecessors: "
215-
<< symbol->getName();
228+
<< OpWithFlags(symbol, OpPrintingFlags().skipRegions());
216229
}
217230
};
218231
SymbolTable::walkSymbolTables(top, /*allSymUsesVisible=*/!top->getBlock(),
@@ -235,7 +248,8 @@ LogicalResult DeadCodeAnalysis::initializeRecursively(Operation *op) {
235248
// Initialize the analysis by visiting every op with control-flow semantics.
236249
if (op->getNumRegions() || op->getNumSuccessors() ||
237250
isRegionOrCallableReturn(op) || isa<CallOpInterface>(op)) {
238-
LDBG() << "[init] Visiting op with control-flow semantics: " << *op;
251+
LDBG() << "[init] Visiting op with control-flow semantics: "
252+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
239253
// When the liveness of the parent block changes, make sure to
240254
// re-invoke the analysis on the op.
241255
if (op->getBlock())
@@ -247,7 +261,8 @@ LogicalResult DeadCodeAnalysis::initializeRecursively(Operation *op) {
247261
}
248262
// Recurse on nested operations.
249263
for (Region &region : op->getRegions()) {
250-
LDBG() << "[init] Recursing into region of op: " << op->getName();
264+
LDBG() << "[init] Recursing into region of op: "
265+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
251266
for (Operation &nestedOp : region.getOps()) {
252267
LDBG() << "[init] Recursing into nested op: "
253268
<< OpWithFlags(&nestedOp, OpPrintingFlags().skipRegions());
@@ -270,14 +285,16 @@ void DeadCodeAnalysis::markEdgeLive(Block *from, Block *to) {
270285
}
271286

272287
void DeadCodeAnalysis::markEntryBlocksLive(Operation *op) {
273-
LDBG() << "Marking entry blocks live for op: " << op->getName();
288+
LDBG() << "Marking entry blocks live for op: "
289+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
274290
for (Region &region : op->getRegions()) {
275291
if (region.empty())
276292
continue;
277293
auto *state =
278294
getOrCreate<Executable>(getProgramPointBefore(&region.front()));
279295
propagateIfChanged(state, state->setToLive());
280-
LDBG() << "Marked entry block live for region in op: " << op->getName();
296+
LDBG() << "Marked entry block live for region in op: "
297+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
281298
}
282299
}
283300

@@ -286,32 +303,37 @@ LogicalResult DeadCodeAnalysis::visit(ProgramPoint *point) {
286303
if (point->isBlockStart())
287304
return success();
288305
Operation *op = point->getPrevOp();
289-
LDBG() << "Visiting operation: " << *op;
306+
LDBG() << "Visiting operation: "
307+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
290308

291309
// If the parent block is not executable, there is nothing to do.
292310
if (op->getBlock() != nullptr &&
293311
!getOrCreate<Executable>(getProgramPointBefore(op->getBlock()))
294312
->isLive()) {
295-
LDBG() << "Parent block not live, skipping op: " << *op;
313+
LDBG() << "Parent block not live, skipping op: "
314+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
296315
return success();
297316
}
298317

299318
// We have a live call op. Add this as a live predecessor of the callee.
300319
if (auto call = dyn_cast<CallOpInterface>(op)) {
301-
LDBG() << "Visiting call operation: " << *op;
320+
LDBG() << "Visiting call operation: "
321+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
302322
visitCallOperation(call);
303323
}
304324

305325
// Visit the regions.
306326
if (op->getNumRegions()) {
307327
// Check if we can reason about the region control-flow.
308328
if (auto branch = dyn_cast<RegionBranchOpInterface>(op)) {
309-
LDBG() << "Visiting region branch operation: " << *op;
329+
LDBG() << "Visiting region branch operation: "
330+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
310331
visitRegionBranchOperation(branch);
311332

312333
// Check if this is a callable operation.
313334
} else if (auto callable = dyn_cast<CallableOpInterface>(op)) {
314-
LDBG() << "Visiting callable operation: " << *op;
335+
LDBG() << "Visiting callable operation: "
336+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
315337
const auto *callsites = getOrCreateFor<PredecessorState>(
316338
getProgramPointAfter(op), getProgramPointAfter(callable));
317339

@@ -323,19 +345,22 @@ LogicalResult DeadCodeAnalysis::visit(ProgramPoint *point) {
323345

324346
// Otherwise, conservatively mark all entry blocks as executable.
325347
} else {
326-
LDBG() << "Marking all entry blocks live for op: " << *op;
348+
LDBG() << "Marking all entry blocks live for op: "
349+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
327350
markEntryBlocksLive(op);
328351
}
329352
}
330353

331354
if (isRegionOrCallableReturn(op)) {
332355
if (auto branch = dyn_cast<RegionBranchOpInterface>(op->getParentOp())) {
333-
LDBG() << "Visiting region terminator: " << *op;
356+
LDBG() << "Visiting region terminator: "
357+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
334358
// Visit the exiting terminator of a region.
335359
visitRegionTerminator(op, branch);
336360
} else if (auto callable =
337361
dyn_cast<CallableOpInterface>(op->getParentOp())) {
338-
LDBG() << "Visiting callable terminator: " << *op;
362+
LDBG() << "Visiting callable terminator: "
363+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
339364
// Visit the exiting terminator of a callable.
340365
visitCallableTerminator(op, callable);
341366
}
@@ -344,12 +369,14 @@ LogicalResult DeadCodeAnalysis::visit(ProgramPoint *point) {
344369
if (op->getNumSuccessors()) {
345370
// Check if we can reason about the control-flow.
346371
if (auto branch = dyn_cast<BranchOpInterface>(op)) {
347-
LDBG() << "Visiting branch operation: " << *op;
372+
LDBG() << "Visiting branch operation: "
373+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
348374
visitBranchOperation(branch);
349375

350376
// Otherwise, conservatively mark all successors as exectuable.
351377
} else {
352-
LDBG() << "Marking all successors live for op: " << *op;
378+
LDBG() << "Marking all successors live for op: "
379+
<< OpWithFlags(op, OpPrintingFlags().skipRegions());
353380
for (Block *successor : op->getSuccessors())
354381
markEdgeLive(op->getBlock(), successor);
355382
}
@@ -359,7 +386,8 @@ LogicalResult DeadCodeAnalysis::visit(ProgramPoint *point) {
359386
}
360387

361388
void DeadCodeAnalysis::visitCallOperation(CallOpInterface call) {
362-
LDBG() << "visitCallOperation: " << call.getOperation()->getName();
389+
LDBG() << "visitCallOperation: "
390+
<< OpWithFlags(call.getOperation(), OpPrintingFlags().skipRegions());
363391
Operation *callableOp = call.resolveCallableInTable(&symbolTable);
364392

365393
// A call to a externally-defined callable has unknown predecessors.
@@ -442,7 +470,8 @@ void DeadCodeAnalysis::visitBranchOperation(BranchOpInterface branch) {
442470

443471
void DeadCodeAnalysis::visitRegionBranchOperation(
444472
RegionBranchOpInterface branch) {
445-
LDBG() << "visitRegionBranchOperation: " << branch.getOperation()->getName();
473+
LDBG() << "visitRegionBranchOperation: "
474+
<< OpWithFlags(branch.getOperation(), OpPrintingFlags().skipRegions());
446475
// Try to deduce which regions are executable.
447476
std::optional<SmallVector<Attribute>> operands = getOperandValues(branch);
448477
if (!operands)
@@ -519,14 +548,14 @@ void DeadCodeAnalysis::visitCallableTerminator(Operation *op,
519548
if (canResolve) {
520549
propagateIfChanged(predecessors, predecessors->join(op));
521550
LDBG() << "Added callable terminator as predecessor for callsite: "
522-
<< predecessor->getName();
551+
<< OpWithFlags(predecessor, OpPrintingFlags().skipRegions());
523552
} else {
524553
// If the terminator is not a return-like, then conservatively assume we
525554
// can't resolve the predecessor.
526555
propagateIfChanged(predecessors,
527556
predecessors->setHasUnknownPredecessors());
528557
LDBG() << "Could not resolve callable terminator for callsite: "
529-
<< predecessor->getName();
558+
<< OpWithFlags(predecessor, OpPrintingFlags().skipRegions());
530559
}
531560
}
532561
}

‎mlir/lib/Analysis/DataFlowFramework.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@ void ProgramPoint::print(raw_ostream &os) const {
6262
return;
6363
}
6464
if (!isBlockStart()) {
65-
os << "<after operation>:";
66-
return getPrevOp()->print(os, OpPrintingFlags().skipRegions());
65+
os << "<after operation>:"
66+
<< OpWithFlags(getPrevOp(), OpPrintingFlags().skipRegions());
67+
return;
6768
}
68-
os << "<before operation>:";
69-
return getNextOp()->print(os, OpPrintingFlags().skipRegions());
69+
os << "<before operation>:"
70+
<< OpWithFlags(getNextOp(), OpPrintingFlags().skipRegions());
7071
}
7172

7273
//===----------------------------------------------------------------------===//

0 commit comments

Comments
 (0)