[mlir] Cleanup dump() functions. #89216
Open
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
Meinersbur
changed the title
Meinersbur
requested review from
ftynse,
nicolasvasilache,
Groverkss,
Superty and
matthias-springer
as code owners
llvmbot
added
mlir:core
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir-linalg Author: Michael Kruse (Meinersbur) ChangesApply the pattern for the dump() function established in 8c209aa to all dump() methods in MLIR.
Patch is 25.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89216.diff 32 Files Affected:
diff --git a/mlir/include/mlir/Analysis/DataFlowFramework.h b/mlir/include/mlir/Analysis/DataFlowFramework.h
index c76cfac07fc77a..3d22928614261f 100644
--- a/mlir/include/mlir/Analysis/DataFlowFramework.h
+++ b/mlir/include/mlir/Analysis/DataFlowFramework.h
@@ -323,7 +323,7 @@ class AnalysisState {
/// Print the contents of the analysis state.
virtual void print(raw_ostream &os) const = 0;
- LLVM_DUMP_METHOD void dump() const;
+ void dump() const;
/// Add a dependency to this analysis state on a program point and an
/// analysis. If this state is updated, the analysis will be invoked on the
diff --git a/mlir/include/mlir/Debug/ExecutionContext.h b/mlir/include/mlir/Debug/ExecutionContext.h
index fbad04b9a2f1d7..49d35a09dd2a2d 100644
--- a/mlir/include/mlir/Debug/ExecutionContext.h
+++ b/mlir/include/mlir/Debug/ExecutionContext.h
@@ -29,10 +29,7 @@ struct ActionActiveStack {
const Action &getAction() const { return action; }
int getDepth() const { return depth; }
void print(raw_ostream &os, bool withContext) const;
- void dump() const {
- print(llvm::errs(), /*withContext=*/true);
- llvm::errs() << "\n";
- }
+ void dump() const;
Breakpoint *getBreakpoint() const { return breakpoint; }
void setBreakpoint(Breakpoint *breakpoint) { this->breakpoint = breakpoint; }
diff --git a/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h b/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
index b8f354892ee60a..3d1c3a80a45805 100644
--- a/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
+++ b/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
@@ -205,7 +205,7 @@ struct MemRefDependenceGraph {
void print(raw_ostream &os) const;
- void dump() const { print(llvm::errs()); }
+ void dump() const;
/// The block for which this graph is created to perform fusion.
Block █
diff --git a/mlir/include/mlir/Dialect/Linalg/TransformOps/GPUHeuristics.h b/mlir/include/mlir/Dialect/Linalg/TransformOps/GPUHeuristics.h
index f9fd32c2031afb..139c390daf67e8 100644
--- a/mlir/include/mlir/Dialect/Linalg/TransformOps/GPUHeuristics.h
+++ b/mlir/include/mlir/Dialect/Linalg/TransformOps/GPUHeuristics.h
@@ -92,7 +92,7 @@ struct CopyMappingInfo : public MappingInfo {
public:
// Pretty-printing and diagnostic methods.
void print(llvm::raw_ostream &os) const;
- LLVM_DUMP_METHOD void dump() const;
+ void dump() const;
/// Static quantity determining the number of bits to target in an individual
/// copy. Assumes that smaller increments of 64, 32, 16, 8 are also valid
diff --git a/mlir/include/mlir/IR/Location.h b/mlir/include/mlir/IR/Location.h
index d4268e804f4f7a..6092f75136ed1e 100644
--- a/mlir/include/mlir/IR/Location.h
+++ b/mlir/include/mlir/IR/Location.h
@@ -96,7 +96,7 @@ class Location {
/// Print the location.
void print(raw_ostream &os) const { impl.print(os); }
- void dump() const { impl.dump(); }
+ void dump() const;
friend ::llvm::hash_code hash_value(Location arg);
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 59f094d6690991..82a20f82fd91bd 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -119,7 +119,7 @@ class OpState {
}
/// Dump this operation.
- void dump() { state->dump(); }
+ void dump() const;
/// The source location the operation was defined or derived from.
Location getLoc() { return state->getLoc(); }
@@ -269,7 +269,7 @@ class OpFoldResult : public PointerUnion<Attribute, Value> {
using PointerUnion<Attribute, Value>::PointerUnion;
public:
- void dump() const { llvm::errs() << *this << "\n"; }
+ void dump() const;
MLIRContext *getContext() const {
return is<Attribute>() ? get<Attribute>().getContext()
diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index c52a6fcac10c1c..6f3af890c9e8b9 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -1019,16 +1019,12 @@ class alignas(8) Operation final
/// Expose a few methods explicitly for the debugger to call for
/// visualization.
-#ifndef NDEBUG
- LLVM_DUMP_METHOD operand_range debug_getOperands() { return getOperands(); }
- LLVM_DUMP_METHOD result_range debug_getResults() { return getResults(); }
- LLVM_DUMP_METHOD SuccessorRange debug_getSuccessors() {
- return getSuccessors();
- }
- LLVM_DUMP_METHOD MutableArrayRef<Region> debug_getRegions() {
- return getRegions();
- }
-#endif
+ /// @{
+ operand_range debug_getOperands();
+ result_range debug_getResults();
+ SuccessorRange debug_getSuccessors();
+ MutableArrayRef<Region> debug_getRegions();
+ /// @}
/// The operation block that contains this operation.
Block *block = nullptr;
diff --git a/mlir/include/mlir/IR/Value.h b/mlir/include/mlir/IR/Value.h
index a74d0faa1dfc4b..fbda9873aa6fd3 100644
--- a/mlir/include/mlir/IR/Value.h
+++ b/mlir/include/mlir/IR/Value.h
@@ -73,11 +73,10 @@ class alignas(8) ValueImpl : public IRObjectWithUseList<OpOperand> {
/// Expose a few methods explicitly for the debugger to call for
/// visualization.
-#ifndef NDEBUG
- LLVM_DUMP_METHOD Type debug_getType() const { return getType(); }
- LLVM_DUMP_METHOD Kind debug_getKind() const { return getKind(); }
-
-#endif
+ /// @{
+ Type debug_getType() const;
+ Kind debug_getKind() const;
+ /// @}
/// The type of this result and the kind.
llvm::PointerIntPair<Type, 3, Kind> typeAndKind;
diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h
index 1b2e6a3bc82bb4..8854ddcbdbc9d7 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -148,7 +148,7 @@ class OpPassManager {
void printAsTextualPipeline(raw_ostream &os) const;
/// Raw dump of the pass manager to llvm::errs().
- void dump();
+ void dump() const;
/// Merge the pass statistics of this class into 'other'.
void mergeStatisticsInto(OpPassManager &other);
diff --git a/mlir/lib/Analysis/CallGraph.cpp b/mlir/lib/Analysis/CallGraph.cpp
index ccd4676632136b..9b0cc80f48465a 100644
--- a/mlir/lib/Analysis/CallGraph.cpp
+++ b/mlir/lib/Analysis/CallGraph.cpp
@@ -174,8 +174,11 @@ void CallGraph::eraseNode(CallGraphNode *node) {
//===----------------------------------------------------------------------===//
// Printing
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Dump the graph in a human readable format.
-void CallGraph::dump() const { print(llvm::errs()); }
+LLVM_DUMP_METHOD void CallGraph::dump() const { print(llvm::errs()); }
+#endif
+
void CallGraph::print(raw_ostream &os) const {
os << "// ---- CallGraph ----\n";
diff --git a/mlir/lib/Analysis/DataFlowFramework.cpp b/mlir/lib/Analysis/DataFlowFramework.cpp
index 5ef24f201f8a67..01bbd59f616dfe 100644
--- a/mlir/lib/Analysis/DataFlowFramework.cpp
+++ b/mlir/lib/Analysis/DataFlowFramework.cpp
@@ -51,7 +51,9 @@ void AnalysisState::addDependency(ProgramPoint dependent,
});
}
-void AnalysisState::dump() const { print(llvm::errs()); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void AnalysisState::dump() const { print(llvm::errs()); }
+#endif
//===----------------------------------------------------------------------===//
// ProgramPoint
diff --git a/mlir/lib/Analysis/Liveness.cpp b/mlir/lib/Analysis/Liveness.cpp
index a8e0daeabf4061..48cbc59611aa7c 100644
--- a/mlir/lib/Analysis/Liveness.cpp
+++ b/mlir/lib/Analysis/Liveness.cpp
@@ -253,8 +253,10 @@ bool Liveness::isDeadAfter(Value value, Operation *operation) const {
return endOperation == operation || endOperation->isBeforeInBlock(operation);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Dumps the liveness information in a human readable format.
-void Liveness::dump() const { print(llvm::errs()); }
+LLVM_DUMP_METHOD void Liveness::dump() const { print(llvm::errs()); }
+#endif
/// Dumps the liveness information to the given stream.
void Liveness::print(raw_ostream &os) const {
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index a3f971db4bd428..82af7285d40271 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -2542,7 +2542,9 @@ void IntegerRelation::print(raw_ostream &os) const {
os << '\n';
}
-void IntegerRelation::dump() const { print(llvm::errs()); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void IntegerRelation::dump() const { print(llvm::errs()); }
+#endif
unsigned IntegerPolyhedron::insertVar(VarKind kind, unsigned pos,
unsigned num) {
diff --git a/mlir/lib/Analysis/Presburger/MPInt.cpp b/mlir/lib/Analysis/Presburger/MPInt.cpp
index 587e2b572facf3..f89a2c93bec813 100644
--- a/mlir/lib/Analysis/Presburger/MPInt.cpp
+++ b/mlir/lib/Analysis/Presburger/MPInt.cpp
@@ -29,7 +29,9 @@ llvm::raw_ostream &MPInt::print(llvm::raw_ostream &os) const {
return os << valLarge;
}
-void MPInt::dump() const { print(llvm::errs()); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void MPInt::dump() const { print(llvm::errs()); }
+#endif
llvm::raw_ostream &mlir::presburger::operator<<(llvm::raw_ostream &os,
const MPInt &x) {
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 4cb6e6b16bc878..e062320c9cd27d 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -422,10 +422,12 @@ Matrix<T>::splitByBitset(ArrayRef<int> indicator) {
return {rowsForOne, rowsForZero};
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
template <typename T>
-void Matrix<T>::dump() const {
+LLVM_DUMP_METHOD void Matrix<T>::dump() const {
print(llvm::errs());
}
+#endif
template <typename T>
bool Matrix<T>::hasConsistentState() const {
diff --git a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
index d55962616de175..39afb584e42aca 100644
--- a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
+++ b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
@@ -67,6 +67,10 @@ void MultiAffineFunction::print(raw_ostream &os) const {
output.print(os);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void MultiAffineFunction::dump() const { print(llvm::errs()); }
+#endif
+
SmallVector<MPInt, 8>
MultiAffineFunction::valueAt(ArrayRef<MPInt> point) const {
assert(point.size() == getNumDomainVars() + getNumSymbolVars() &&
@@ -309,7 +313,10 @@ void PWMAFunction::print(raw_ostream &os) const {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void PWMAFunction::dump() const { print(llvm::errs()); }
+#endif
PWMAFunction PWMAFunction::unionFunction(
const PWMAFunction &func,
diff --git a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
index 3af6baae0e7001..e021c304214bdd 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
@@ -1055,7 +1055,10 @@ void PresburgerRelation::print(raw_ostream &os) const {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void PresburgerRelation::dump() const { print(llvm::errs()); }
+#endif
PresburgerSet PresburgerSet::getUniverse(const PresburgerSpace &space) {
PresburgerSet result(space);
diff --git a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
index f80df52fb82908..4ac5a504fd5881 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
@@ -28,10 +28,13 @@ void Identifier::print(llvm::raw_ostream &os) const {
os << "Id<" << value << ">";
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void Identifier::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
PresburgerSpace PresburgerSpace::getDomainSpace() const {
PresburgerSpace newSpace = *this;
@@ -351,7 +354,10 @@ void PresburgerSpace::print(llvm::raw_ostream &os) const {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void PresburgerSpace::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 1969cce93ad2e0..7964d60a8d561e 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -2152,7 +2152,10 @@ void SimplexBase::print(raw_ostream &os) const {
os << '\n';
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void SimplexBase::dump() const { print(llvm::errs()); }
+#endif
bool Simplex::isRationalSubsetOf(const IntegerRelation &rel) {
if (isEmpty())
diff --git a/mlir/lib/Analysis/Presburger/SlowMPInt.cpp b/mlir/lib/Analysis/Presburger/SlowMPInt.cpp
index ae6f2827be9268..7c81bfdf2672ec 100644
--- a/mlir/lib/Analysis/Presburger/SlowMPInt.cpp
+++ b/mlir/lib/Analysis/Presburger/SlowMPInt.cpp
@@ -36,7 +36,10 @@ llvm::hash_code detail::hash_value(const SlowMPInt &x) {
/// ---------------------------------------------------------------------------
void SlowMPInt::print(llvm::raw_ostream &os) const { os << val; }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void SlowMPInt::dump() const { print(llvm::errs()); }
+#endif
llvm::raw_ostream &detail::operator<<(llvm::raw_ostream &os,
const SlowMPInt &x) {
diff --git a/mlir/lib/Analysis/Presburger/Utils.cpp b/mlir/lib/Analysis/Presburger/Utils.cpp
index f717a4de5d7283..7db638bebe4148 100644
--- a/mlir/lib/Analysis/Presburger/Utils.cpp
+++ b/mlir/lib/Analysis/Presburger/Utils.cpp
@@ -516,7 +516,10 @@ void DivisionRepr::print(raw_ostream &os) const {
os << "\n";
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void DivisionRepr::dump() const { print(llvm::errs()); }
+#endif
SmallVector<MPInt, 8> presburger::getMPIntVec(ArrayRef<int64_t> range) {
SmallVector<MPInt, 8> result(range.size());
diff --git a/mlir/lib/Debug/ExecutionContext.cpp b/mlir/lib/Debug/ExecutionContext.cpp
index f7505b6608c814..a4e884279ce86c 100644
--- a/mlir/lib/Debug/ExecutionContext.cpp
+++ b/mlir/lib/Debug/ExecutionContext.cpp
@@ -44,6 +44,14 @@ void ActionActiveStack::print(raw_ostream &os, bool withContext) const {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
+void ActionActiveStack::dump() const {
+ print(llvm::errs(), /*withContext=*/true);
+ llvm::errs() << "\n";
+}
+#endif
+
//===----------------------------------------------------------------------===//
// ExecutionContext
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/Affine/Analysis/Utils.cpp b/mlir/lib/Dialect/Affine/Analysis/Utils.cpp
index 194ee9115e3d7a..667bcd8f56b01a 100644
--- a/mlir/lib/Dialect/Affine/Analysis/Utils.cpp
+++ b/mlir/lib/Dialect/Affine/Analysis/Utils.cpp
@@ -630,6 +630,11 @@ void MemRefDependenceGraph::print(raw_ostream &os) const {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
+void MemRefDependenceGraph::dump() const { print(llvm::errs()); }
+#endif
+
void mlir::affine::getAffineForIVs(Operation &op,
SmallVectorImpl<AffineForOp> *loops) {
auto *currOp = op.getParentOp();
@@ -720,6 +725,8 @@ void ComputationSliceState::clearBounds() {
ubOperands.clear();
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void ComputationSliceState::dump() const {
llvm::errs() << "\tIVs:\n";
for (Value iv : ivs)
@@ -741,6 +748,7 @@ void ComputationSliceState::dump() const {
llvm::errs() << "\t\t\t" << ubOp << "\n";
}
}
+#endif
/// Fast check to determine if the computation slice is maximal. Returns true if
/// each slice dimension maps to an existing dst dimension and both the src
diff --git a/mlir/lib/Dialect/Linalg/TransformOps/GPUHeuristics.cpp b/mlir/lib/Dialect/Linalg/TransformOps/GPUHeuristics.cpp
index 38abdd122d633d..3c807c386a24e4 100644
--- a/mlir/lib/Dialect/Linalg/TransformOps/GPUHeuristics.cpp
+++ b/mlir/lib/Dialect/Linalg/TransformOps/GPUHeuristics.cpp
@@ -265,3 +265,8 @@ void transform::gpu::CopyMappingInfo::print(llvm::raw_ostream &os) const {
llvm::interleaveComma(threadMapping, os << "}, threadMapping: {");
os << "}}";
}
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
+void transform::gpu::CopyMappingInfo::dump() const { print(llvm::errs()); }
+#endif
\ No newline at end of file
diff --git a/mlir/lib/Dialect/Polynomial/IR/Polynomial.cpp b/mlir/lib/Dialect/Polynomial/IR/Polynomial.cpp
index 5916ffba78e246..3deeb92659ae24 100644
--- a/mlir/lib/Dialect/Polynomial/IR/Polynomial.cpp
+++ b/mlir/lib/Dialect/Polynomial/IR/Polynomial.cpp
@@ -81,6 +81,11 @@ void Polynomial::print(raw_ostream &os, ::llvm::StringRef separator,
void Polynomial::print(raw_ostream &os) const { print(os, " + ", "**"); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
+void Polynomial::dump() const { print(llvm::errs()); }
+#endif
+
std::string Polynomial::toIdentifier() const {
std::string result;
llvm::raw_string_ostream os(result);
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index e915b97d9ff17b..b2bce97d024380 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -59,7 +59,10 @@ using namespace mlir::detail;
void OperationName::print(raw_ostream &os) const { os << getStringRef(); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void OperationName::dump() const { print(llvm::errs()); }
+#endif
//===--------------------------------------------------------------------===//
// AsmParser
@@ -3744,10 +3747,13 @@ void Attribute::print(raw_ostream &os, AsmState &state, bool elideType) const {
: AttrTypeElision::Never);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void Attribute::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
void Attribute::printStripped(raw_ostream &os, AsmState &state) const {
if (!*this) {
@@ -3793,20 +3799,25 @@ void Type::print(raw_ostream &os, AsmState &state) const {
AsmPrinter::Impl(os, state.getImpl()).printType(*this);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void Type::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+LLVM_DUMP_METHOD
void AffineMap::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+LLVM_DUMP_METHOD
void IntegerSet::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
void AffineExpr::print(raw_ostream &os) const {
if (!expr) {
@@ -3817,10 +3828,13 @@ void AffineExpr::print(raw_ostream &os) const {
AsmPrinter::Impl(os, state.getImpl()).printAffineExpr(*this);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void AffineExpr::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
void AffineMap::print(raw_ostream &os) const {
if (!map) {
@@ -3865,10 +3879,13 @@ void Value::print(raw_ostream &os, AsmState &state) const {
<< "' at index: " << arg.getArgNumber();
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void Value::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
void Value::printAsOperand(raw_ostream &os, AsmState &state) const {
// TODO: This doesn't necessarily capture all potential cases.
@@ -3928,10 +3945,12 @@ void Operation::print(raw_ostream &os, AsmState &state) {
}
}
-void Operation::dump() {
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void Operation::dump() {
print(llvm::errs(), OpPrintingFlags().useLocalScope());
llvm::errs() << "\n";
}
+#endif
void Block::print(raw_ostream &os) {
Operation *parentOp = getParentOp();
@@ -3950,7 +3969,10 @@ void Block::print(raw_ostream &os, AsmState &state) {
OperationPrinter(os, state.getImpl()).print(this);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD
void Block::dump() { print(llvm::errs()); }
+#...
[truncated]
|
|
I don't understand much here, but does this mean |
joker-eph
reviewed
Apr 20, 2024
| } | ||
| #endif | ||
| /// @{ | ||
| operand_range debug_getOperands(); |
There was a problem hiding this comment.
This makes it possible to turn a compile-time failure into a link-time failure, that seems like a "regression" to me in behavior.
You mention a problem of flag divergence, however that's what llvm-config.h is here to resolve.
And actually there is a FIXME before the definition of LLVM_DUMP_METHOD to this effect:
// FIXME: Move this to a private config.h as it's not usable in public headers.
There was a problem hiding this comment.
So the FIXME was fixed by llvm-config.h?
However, the LLVM_DUMP_METHOD makes use of the NDEBUG macro which is under the including application's control and LLVM_ENABLE_DEBUG is not present in llvm-config.h.
This makes it possible to turn a compile-time failure into a link-time failure, that seems like a "regression" to me in behavior.
This is by design of the LLVM_DUMP_METHOD pattern established in 8c209aa. I think there the amount of bloat if we also emit dump functions in release builds would be insignificant. Or, dump functions are only controlled by LLVM_ENABLE_DEBUG only, conditionally compiled in header and translation unit, and also emitted into llvm-config.h. But I think such a change should be LLVM-wide.
I guess one could make some inline dump functions be including-application controlled by user application macros. However, the cost is high: because by design there are no uses of such functions, __attribute__((__used__)) must be used which causes it to be emitted in every single translation unit. I'd favor to emit dump function into release build libraries.
Anyway, there should be a correct use of the LLVM_DUMP_METHOD macro, and MLIR currently doesn't follow any pattern.
There was a problem hiding this comment.
So the FIXME was fixed by llvm-config.h? However, the LLVM_DUMP_METHOD makes use of the NDEBUG macro which is under the including application's control and LLVM_ENABLE_DEBUG is not present in llvm-config.h.
No it's not fixed, it should be fixed by making use of llvm-config.h to not rely on NDEBUG.
This is by design of the LLVM_DUMP_METHOD pattern established in 8c209aa.
I don't think the aspect of "not failing at compile time" is actually desirable "by design" here, why would it be better than relying on llvm-config.h ?
There was a problem hiding this comment.
It's not about desirability, but following LLVM coding practice.
That can be changed of course, but will require changes through all of LLVM (due to changing the LLVM_DUMP_METHOD macro) and possibly require a community discussion. In the meantime, this would be the fix for making most dump() methods actually callable in the debugger. I'd add the suggested usability improvement to not declare those functions depending on llvm-config.h in a followup.
There was a problem hiding this comment.
Sorry, I don't agree that this is a "coding practice" just because this is a pattern appearing somewhere else.
In particular the existing FIXME in the code acknowledge that this isn't how it should be setup, and I don't understand why changing this as you do right now is a good thing to do, instead of addressing the FIXME.
I don't follow why fixing the FIXME would require immediately changes through all of LLVM: it seems to me that this can be changed in a compatible way.
Yes, this fixes that. for variables of type |
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.
Apply the pattern for the dump() function established in 8c209aa to all dump() methods in MLIR.
constwhere appropriate.