mlir/Presburger: thoroughly guard debug-print #95247
Closed
Conversation
This file contains 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
Follow up on 638d968 (mlir/Presburger: guard dump function; fix buildbot) to implement a more thorough fix. Guard all debug-printing functions in Presburger with !NDEBUG || LLVM_ENABLE_DUMP. This fixes the build on one particular configuration: without assertions, and LLVM_ENABLE_DUMP explicitly turned off.
artagnon
requested review from
Groverkss,
Superty and
matthias-springer
as code owners
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-presburger Author: Ramkumar Ramachandra (artagnon) ChangesFollow up on 638d968 (mlir/Presburger: guard dump function; fix buildbot) to implement a more thorough fix. Guard all debug-printing functions in Presburger with !NDEBUG || LLVM_ENABLE_DUMP. This fixes the build on one particular configuration: without assertions, and LLVM_ENABLE_DUMP explicitly turned off. Full diff: https://github.com/llvm/llvm-project/pull/95247.diff 21 Files Affected:
diff --git a/mlir/include/mlir/Analysis/FlatLinearValueConstraints.h b/mlir/include/mlir/Analysis/FlatLinearValueConstraints.h
index cc6ab64b4b7d7..4019a1c80d2b8 100644
--- a/mlir/include/mlir/Analysis/FlatLinearValueConstraints.h
+++ b/mlir/include/mlir/Analysis/FlatLinearValueConstraints.h
@@ -218,10 +218,12 @@ class FlatLinearConstraints : public presburger::IntegerPolyhedron {
AffineMap map, std::vector<SmallVector<int64_t, 8>> *flattenedExprs,
bool addConservativeSemiAffineBounds = false);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Prints the number of constraints, dimensions, symbols and locals in the
/// FlatLinearConstraints. Also, prints for each variable whether there is
/// an SSA Value attached to it.
void printSpace(raw_ostream &os) const override;
+#endif
};
/// FlatLinearValueConstraints represents an extension of FlatLinearConstraints
@@ -432,10 +434,12 @@ class FlatLinearValueConstraints : public FlatLinearConstraints {
void projectOut(Value val);
using IntegerPolyhedron::projectOut;
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Prints the number of constraints, dimensions, symbols and locals in the
/// FlatAffineValueConstraints. Also, prints for each variable whether there
/// is an SSA Value attached to it.
void printSpace(raw_ostream &os) const override;
+#endif
/// Align `map` with this constraint system based on `operands`. Each operand
/// must already have a corresponding dim/symbol in this constraint system.
diff --git a/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h b/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
index db5b6b6a95918..9da6e9e2f5638 100644
--- a/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
+++ b/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
@@ -90,6 +90,7 @@ class GeneratingFunction {
sumDenominators);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
llvm::raw_ostream &print(llvm::raw_ostream &os) const {
for (unsigned i = 0, e = signs.size(); i < e; i++) {
if (i == 0) {
@@ -124,6 +125,7 @@ class GeneratingFunction {
}
return os;
}
+#endif
private:
unsigned numParam;
diff --git a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
index 40e96e2583d22..b7274778c9917 100644
--- a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
@@ -748,8 +748,10 @@ class IntegerRelation {
// false.
bool isFullDim();
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream &os) const;
void dump() const;
+#endif
protected:
/// Checks all rows of equality/inequality constraints for trivial
@@ -834,9 +836,11 @@ class IntegerRelation {
/// is meant to be used within an assert internally.
virtual bool hasConsistentState() const;
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Prints the number of constraints, dimensions, symbols and locals in the
/// IntegerRelation.
virtual void printSpace(raw_ostream &os) const;
+#endif
/// Removes variables in the column range [varStart, varLimit), and copies any
/// remaining valid data into place, updates member variables, and resizes
diff --git a/mlir/include/mlir/Analysis/Presburger/Matrix.h b/mlir/include/mlir/Analysis/Presburger/Matrix.h
index e232ecd5e1509..397eacb6dd36f 100644
--- a/mlir/include/mlir/Analysis/Presburger/Matrix.h
+++ b/mlir/include/mlir/Analysis/Presburger/Matrix.h
@@ -206,9 +206,11 @@ class Matrix {
/// corresponding to 2.
std::pair<Matrix<T>, Matrix<T>> splitByBitset(ArrayRef<int> indicator);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the matrix.
void print(raw_ostream &os) const;
void dump() const;
+#endif
/// Return whether the Matrix is in a consistent state with all its
/// invariants satisfied.
diff --git a/mlir/include/mlir/Analysis/Presburger/PWMAFunction.h b/mlir/include/mlir/Analysis/Presburger/PWMAFunction.h
index fcc39bf0e0537..bc9d7f31a8fa7 100644
--- a/mlir/include/mlir/Analysis/Presburger/PWMAFunction.h
+++ b/mlir/include/mlir/Analysis/Presburger/PWMAFunction.h
@@ -109,8 +109,10 @@ class MultiAffineFunction {
/// Get this function as a relation.
IntegerRelation getAsRelation() const;
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream &os) const;
void dump() const;
+#endif
private:
/// Assert that the MAF is consistent.
@@ -220,8 +222,10 @@ class PWMAFunction {
PWMAFunction unionLexMin(const PWMAFunction &func);
PWMAFunction unionLexMax(const PWMAFunction &func);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream &os) const;
void dump() const;
+#endif
private:
/// Return a function defined on the union of the domains of `this` and
diff --git a/mlir/include/mlir/Analysis/Presburger/PresburgerRelation.h b/mlir/include/mlir/Analysis/Presburger/PresburgerRelation.h
index f7e06a6b22a95..fed2deffdecc0 100644
--- a/mlir/include/mlir/Analysis/Presburger/PresburgerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/PresburgerRelation.h
@@ -221,9 +221,11 @@ class PresburgerRelation {
/// dimensional we mean that it is not flat along any dimension.
bool isFullDim() const;
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the set's internal state.
void print(raw_ostream &os) const;
void dump() const;
+#endif
protected:
/// Construct an empty PresburgerRelation with the specified number of
diff --git a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
index a8ec373f885e2..4fa8455b67460 100644
--- a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
+++ b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
@@ -99,8 +99,10 @@ class Identifier {
bool operator==(const Identifier &other) const { return isEqual(other); }
bool operator!=(const Identifier &other) const { return !isEqual(other); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(llvm::raw_ostream &os) const;
void dump() const;
+#endif
private:
/// The value of the identifier.
@@ -308,8 +310,10 @@ class PresburgerSpace {
/// the same identifiers in the same order.
void mergeAndAlignSymbols(PresburgerSpace &other);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(llvm::raw_ostream &os) const;
void dump() const;
+#endif
protected:
PresburgerSpace(unsigned numDomain, unsigned numRange, unsigned numSymbols,
diff --git a/mlir/include/mlir/Analysis/Presburger/Simplex.h b/mlir/include/mlir/Analysis/Presburger/Simplex.h
index ff26e94e019c8..1a2b1f0f092f6 100644
--- a/mlir/include/mlir/Analysis/Presburger/Simplex.h
+++ b/mlir/include/mlir/Analysis/Presburger/Simplex.h
@@ -208,9 +208,11 @@ class SimplexBase {
/// Add all the constraints from the given IntegerRelation.
void intersectIntegerRelation(const IntegerRelation &rel);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the tableau's internal state.
void print(raw_ostream &os) const;
void dump() const;
+#endif
protected:
/// Construct a SimplexBase with the specified number of variables and fixed
@@ -246,12 +248,14 @@ class SimplexBase {
bool restricted : 1;
bool isSymbol : 1;
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream &os) const {
os << (orientation == Orientation::Row ? "r" : "c");
os << pos;
if (restricted)
os << " [>=0]";
}
+#endif
};
struct Pivot {
diff --git a/mlir/include/mlir/Analysis/Presburger/Utils.h b/mlir/include/mlir/Analysis/Presburger/Utils.h
index 9b93e52b48490..8f34d2821dd7c 100644
--- a/mlir/include/mlir/Analysis/Presburger/Utils.h
+++ b/mlir/include/mlir/Analysis/Presburger/Utils.h
@@ -181,8 +181,10 @@ class DivisionRepr {
void
removeDuplicateDivs(llvm::function_ref<bool(unsigned i, unsigned j)> merge);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream &os) const;
void dump() const;
+#endif
private:
/// Each row of the Matrix represents a single division dividend. The
diff --git a/mlir/include/mlir/Interfaces/ValueBoundsOpInterface.h b/mlir/include/mlir/Interfaces/ValueBoundsOpInterface.h
index 337314143c80c..28ba5a34b02cb 100644
--- a/mlir/include/mlir/Interfaces/ValueBoundsOpInterface.h
+++ b/mlir/include/mlir/Interfaces/ValueBoundsOpInterface.h
@@ -301,9 +301,11 @@ class ValueBoundsConstraintSet
/// Return an expression that represents a constant.
AffineExpr getExpr(int64_t constant);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Debugging only: Dump the constraint set and the column-to-value/dim
/// mapping to llvm::errs.
void dump() const;
+#endif
protected:
/// Dimension identifier to indicate a value is index-typed. This is used for
diff --git a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
index e628fb152b52f..5a6dc34f27d59 100644
--- a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
+++ b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
@@ -1194,6 +1194,7 @@ void FlatLinearValueConstraints::addBound(BoundType type, Value val,
addBound(type, pos, value);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void FlatLinearConstraints::printSpace(raw_ostream &os) const {
IntegerPolyhedron::printSpace(os);
os << "(";
@@ -1221,6 +1222,7 @@ void FlatLinearValueConstraints::printSpace(raw_ostream &os) const {
os << "Local\t";
os << "const)\n";
}
+#endif
void FlatLinearValueConstraints::projectOut(Value val) {
unsigned pos;
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 75215fbab5282..f26f636d69dbd 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -2500,11 +2500,6 @@ void IntegerRelation::applyDomain(const IntegerRelation &rel) {
void IntegerRelation::applyRange(const IntegerRelation &rel) { compose(rel); }
-void IntegerRelation::printSpace(raw_ostream &os) const {
- space.print(os);
- os << getNumConstraints() << " constraints\n";
-}
-
void IntegerRelation::removeTrivialEqualities() {
for (int i = getNumEqualities() - 1; i >= 0; --i)
if (rangeIsZero(getEquality(i)))
@@ -2589,6 +2584,12 @@ void IntegerRelation::mergeAndCompose(const IntegerRelation &other) {
*this = result;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void IntegerRelation::printSpace(raw_ostream &os) const {
+ space.print(os);
+ os << getNumConstraints() << " constraints\n";
+}
+
void IntegerRelation::print(raw_ostream &os) const {
assert(hasConsistentState());
printSpace(os);
@@ -2610,7 +2611,7 @@ void IntegerRelation::print(raw_ostream &os) const {
}
void IntegerRelation::dump() const { print(llvm::errs()); }
-
+#endif
unsigned IntegerPolyhedron::insertVar(VarKind kind, unsigned pos,
unsigned num) {
assert((kind != VarKind::Domain || num == 0) &&
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 134b805648d9f..1dfcd5ce3b166 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -397,6 +397,7 @@ Matrix<T> Matrix<T>::getSubMatrix(unsigned fromRow, unsigned toRow,
return subMatrix;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
template <typename T>
void Matrix<T>::print(raw_ostream &os) const {
for (unsigned row = 0; row < nRows; ++row) {
@@ -406,6 +407,12 @@ void Matrix<T>::print(raw_ostream &os) const {
}
}
+template <typename T>
+void Matrix<T>::dump() const {
+ print(llvm::errs());
+}
+#endif
+
/// We iterate over the `indicator` bitset, checking each bit. If a bit is 1,
/// we append it to one matrix, and if it is zero, we append it to the other.
template <typename T>
@@ -421,11 +428,6 @@ Matrix<T>::splitByBitset(ArrayRef<int> indicator) {
return {rowsForOne, rowsForZero};
}
-template <typename T>
-void Matrix<T>::dump() const {
- print(llvm::errs());
-}
-
template <typename T>
bool Matrix<T>::hasConsistentState() const {
if (data.size() != nRows * nReservedColumns)
diff --git a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
index 664670d506d53..76352fabdfd74 100644
--- a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
+++ b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
@@ -58,6 +58,7 @@ PresburgerSet PWMAFunction::getDomain() const {
return domain;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void MultiAffineFunction::print(raw_ostream &os) const {
space.print(os);
os << "Division Representation:\n";
@@ -65,6 +66,7 @@ void MultiAffineFunction::print(raw_ostream &os) const {
os << "Output:\n";
output.print(os);
}
+#endif
SmallVector<DynamicAPInt, 8>
MultiAffineFunction::valueAt(ArrayRef<DynamicAPInt> point) const {
@@ -299,6 +301,7 @@ void PWMAFunction::addPiece(const Piece &piece) {
pieces.push_back(piece);
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void PWMAFunction::print(raw_ostream &os) const {
space.print(os);
os << getNumPieces() << " pieces:\n";
@@ -311,6 +314,7 @@ void PWMAFunction::print(raw_ostream &os) const {
}
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 6173f774d0475..37ec90e6eeb14 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
@@ -1049,6 +1049,7 @@ bool PresburgerRelation::isFullDim() const {
});
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void PresburgerRelation::print(raw_ostream &os) const {
os << "Number of Disjuncts: " << getNumDisjuncts() << "\n";
for (const IntegerRelation &disjunct : disjuncts) {
@@ -1058,6 +1059,7 @@ void PresburgerRelation::print(raw_ostream &os) const {
}
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 f00796833482e..9dbc7c9bebae6 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
@@ -24,6 +24,7 @@ bool Identifier::isEqual(const Identifier &other) const {
return value == other.value;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void Identifier::print(llvm::raw_ostream &os) const {
os << "Id<" << value << ">";
}
@@ -32,6 +33,7 @@ void Identifier::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
+#endif
PresburgerSpace PresburgerSpace::getDomainSpace() const {
PresburgerSpace newSpace = *this;
@@ -324,6 +326,7 @@ void PresburgerSpace::mergeAndAlignSymbols(PresburgerSpace &other) {
}
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void PresburgerSpace::print(llvm::raw_ostream &os) const {
os << "Domain: " << getNumDomainVars() << ", "
<< "Range: " << getNumRangeVars() << ", "
@@ -356,3 +359,4 @@ 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 2cdd79d42732d..bf0d0a2cdf979 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -2121,6 +2121,7 @@ bool Simplex::isFlatAlong(ArrayRef<DynamicAPInt> coeffs) {
return *upOpt == *downOpt;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void SimplexBase::print(raw_ostream &os) const {
os << "rows = " << getNumRows() << ", columns = " << getNumColumns() << "\n";
if (empty)
@@ -2157,6 +2158,7 @@ void SimplexBase::print(raw_ostream &os) const {
}
void SimplexBase::dump() const { print(llvm::errs()); }
+#endif
bool Simplex::isRationalSubsetOf(const IntegerRelation &rel) {
if (isEmpty())
diff --git a/mlir/lib/Analysis/Presburger/Utils.cpp b/mlir/lib/Analysis/Presburger/Utils.cpp
index 1fab4c4dcca33..4d7b4c4d89712 100644
--- a/mlir/lib/Analysis/Presburger/Utils.cpp
+++ b/mlir/lib/Analysis/Presburger/Utils.cpp
@@ -513,6 +513,7 @@ void DivisionRepr::insertDiv(unsigned pos, unsigned num) {
denoms.insert(denoms.begin() + pos, num, DynamicAPInt(0));
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void DivisionRepr::print(raw_ostream &os) const {
os << "Dividends:\n";
dividends.print(os);
@@ -523,6 +524,7 @@ void DivisionRepr::print(raw_ostream &os) const {
}
void DivisionRepr::dump() const { print(llvm::errs()); }
+#endif
SmallVector<DynamicAPInt, 8>
presburger::getDynamicAPIntVec(ArrayRef<int64_t> range) {
diff --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index 6420c192b257d..cdf130b1843a8 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -881,6 +881,7 @@ ValueBoundsConstraintSet::areEquivalentSlices(MLIRContext *ctx,
return true;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void ValueBoundsConstraintSet::dump() const {
llvm::errs() << "==========\nColumns:\n";
llvm::errs() << "(column\tdim\tvalue)\n";
@@ -909,6 +910,7 @@ void ValueBoundsConstraintSet::dump() const {
cstr.dump();
llvm::errs() << "==========\n";
}
+#endif
ValueBoundsConstraintSet::BoundBuilder &
ValueBoundsConstraintSet::BoundBuilder::operator[](int64_t dim) {
diff --git a/mlir/unittests/Analysis/Presburger/IntegerPolyhedronTest.cpp b/mlir/unittests/Analysis/Presburger/IntegerPolyhedronTest.cpp
index f64bb240b4ee4..e700729bf6616 100644
--- a/mlir/unittests/Analysis/Presburger/IntegerPolyhedronTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/IntegerPolyhedronTest.cpp
@@ -41,11 +41,13 @@ makeSetFromConstraints(unsigned ids, ArrayRef<SmallVector<int64_t, 4>> ineqs,
return set;
}
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
static void dump(ArrayRef<DynamicAPInt> vec) {
for (const DynamicAPInt &x : vec)
llvm::errs() << x << ' ';
llvm::errs() << '\n';
}
+#endif
/// If fn is TestFunction::Sample (default):
///
@@ -69,16 +71,19 @@ static void checkSample(bool hasSample, const IntegerPolyhedron &poly,
if (!hasSample) {
EXPECT_FALSE(maybeSample.has_value());
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
if (maybeSample.has_value()) {
llvm::errs() << "findIntegerSample gave sample: ";
dump(*maybeSample);
}
-
+#endif
EXPECT_TRUE(maybeLexMin.isEmpty());
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
if (maybeLexMin.isBounded()) {
llvm::errs() << "findIntegerLexMin gave sample: ";
dump(*maybeLexMin);
}
+#endif
} else {
ASSERT_TRUE(maybeSample.has_value());
EXPECT_TRUE(poly.containsPoint(*maybeSample));
diff --git a/mlir/unittests/Analysis/Presburger/LinearTransformTest.cpp b/mlir/unittests/Analysis/Presburger/LinearTransformTest.cpp
index 388ac1174dcdc..d76e08a513932 100644
--- a/mlir/unittests/Analysis/Presburger/LinearTransformTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/LinearTransformTest.cpp
@@ -28,10 +28,12 @@ void testColumnEchelonForm(const IntMatrix &m, unsigned expectedRank) {
for (unsigned col = lastAllowedNonZeroCol + 1, nCols = m.getNumColumns();
col < nCols; ++col) {
EXPECT_EQ(rowVec[col], 0);
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
if (rowVec[col] != 0) {
llvm::errs() << "Failed at input matrix:\n";
m.dump();
}
+#endif
}
if (rowVec[lastAllowedNonZeroCol] != 0)
lastAllowedNonZeroCol++;
|
makslevental
requested changes
Jun 12, 2024
There was a problem hiding this comment.
Please send this as a branch on the main repo so that I can test downstream.
Since #95244 reverts the original, please fold this into your reland and thoroughly test it before resubmitting.
|
Do you have some presubmit results showing that this finally fixes the issue? I am not familiar with how to trigger the right buildbots that would check this, maybe you know this? |
|
We shouldn't review PR for build fixes: either this is a trivial fix forward and you just land it ASAP or it isn't and we instead revert the original one in the meantime to unbreak the CI.
FYI: this isn't an appropriate use of branches in the main repo which are documented to be for stacked PR purposes exclusively. |
I will land the revert #95244 as soon as the windows build passes.
Fair enough. I leave it to @artagnon to figure out how to verify thoroughly that the reland doesn't break release builds. |
|
I don’t know of a way. I’ve built with LLVM_ENABLE_ASSERTIONS=off, CMAKE_BUILD_TYPE=Release, and ENABLE_LLVM_DUMP=off, verified that I get a build error before the patch, and that the build error is gone after the patch. Apologies again for breaking the build and the failed fix-forward: I really didn’t think it was an issue.
…On Wed, Jun 12, 2024, at 2:26 PM, Adrian Kuegel wrote:
Do you have some presubmit results showing that this finally fixes the issue? I am not familiar with how to trigger the right buildbots that would check this, maybe you know this?
—
Reply to this email directly, view it on GitHub <#95247 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAJC2WIV2TQLCPDENP2OB3ZHBD7DAVCNFSM6AAAAABJGLRDTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRTGAYDOMRUGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
At this point, I think it would make more sense to instead unconditionally provide |
That's fair. I'll do that in the re-land. |
FYI for the same reason, pure reverts can be just pushed directly instead (I don't even open a PR in general if one of the bot I maintain is broken). |
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.
Follow up on 638d968 (mlir/Presburger: guard dump function; fix buildbot) to implement a more thorough fix. Guard all debug-printing functions in Presburger with !NDEBUG || LLVM_ENABLE_DUMP. This fixes the build on one particular configuration: without assertions, and LLVM_ENABLE_DUMP explicitly turned off.