Skip to content

[mlir] Cleanup dump() functions. #89216

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[mlir] Cleanup dump() functions. #89216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mlir/include/mlir/Analysis/DataFlowFramework.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions mlir/include/mlir/Debug/ExecutionContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &block;
Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/Dialect/Linalg/TransformOps/GPUHeuristics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/IR/Location.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions mlir/include/mlir/IR/OpDefinition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down Expand Up @@ -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()
Expand Down
16 changes: 6 additions & 10 deletions mlir/include/mlir/IR/Operation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@Meinersbur Meinersbur Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@joker-eph joker-eph Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

result_range debug_getResults();
SuccessorRange debug_getSuccessors();
MutableArrayRef<Region> debug_getRegions();
/// @}

/// The operation block that contains this operation.
Block *block = nullptr;
Expand Down
9 changes: 4 additions & 5 deletions mlir/include/mlir/IR/Value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/Pass/PassManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion mlir/lib/Analysis/CallGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
4 changes: 3 additions & 1 deletion mlir/lib/Analysis/DataFlowFramework.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion mlir/lib/Analysis/Liveness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion mlir/lib/Analysis/Presburger/IntegerRelation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion mlir/lib/Analysis/Presburger/MPInt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion mlir/lib/Analysis/Presburger/Matrix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions mlir/lib/Analysis/Presburger/PWMAFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions mlir/lib/Analysis/Presburger/Simplex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
3 changes: 3 additions & 0 deletions mlir/lib/Analysis/Presburger/SlowMPInt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions mlir/lib/Analysis/Presburger/Utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
8 changes: 8 additions & 0 deletions mlir/lib/Debug/ExecutionContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down
8 changes: 8 additions & 0 deletions mlir/lib/Dialect/Affine/Analysis/Utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions mlir/lib/Dialect/Linalg/TransformOps/GPUHeuristics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions mlir/lib/Dialect/Polynomial/IR/Polynomial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading