Skip to content

Commit a24c468

Browse files
authored
[MLIR] Fix assert expressions (llvm#112474)
I noticed that several assertions in MLIR codebase have issues with operator precedence The issue with operator precedence in these assertions is due to the way logical operators are evaluated. The `&&` operator has higher precedence than the `||` operator, which means the assertion is currently evaluating incorrectly, like this: ``` assert((resType.getNumDynamicDims() == dynOutDims.size()) || (dynOutDims.empty() && "Either none or all output dynamic dims must be specified!")); ``` We should add parentheses around the entire expression involving `dynOutDims.empty()` to ensure that the logical conditions are grouped correctly. Here’s the corrected version: ``` assert(((resType.getNumDynamicDims() == dynOutDims.size()) || dynOutDims.empty()) && "Either none or all output dynamic dims must be specified!"); ```
1 parent 0a53f43 commit a24c468

File tree

3 files changed

+10
-10
lines changed

3 files changed

+10
-10
lines changed

mlir/lib/Analysis/FlatLinearValueConstraints.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,8 @@ FlatLinearValueConstraints::FlatLinearValueConstraints(IntegerSet set,
892892
set.getNumDims() + set.getNumSymbols() + 1,
893893
set.getNumDims(), set.getNumSymbols(),
894894
/*numLocals=*/0) {
895-
assert(operands.empty() ||
896-
set.getNumInputs() == operands.size() && "operand count mismatch");
895+
assert((operands.empty() || set.getNumInputs() == operands.size()) &&
896+
"operand count mismatch");
897897
// Set the values for the non-local variables.
898898
for (unsigned i = 0, e = operands.size(); i < e; ++i)
899899
setValue(i, operands[i]);

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -840,11 +840,11 @@ enum VectorMemoryAccessKind { ScalarBroadcast, Contiguous, Gather };
840840
/// TODO: Statically shaped loops + vector masking
841841
static uint64_t getTrailingNonUnitLoopDimIdx(LinalgOp linalgOp) {
842842
SmallVector<int64_t> loopRanges = linalgOp.getStaticLoopRanges();
843-
assert(linalgOp.hasDynamicShape() ||
844-
llvm::count_if(loopRanges, [](int64_t dim) { return dim != 1; }) ==
845-
1 &&
846-
"For statically shaped Linalg Ops, only one "
847-
"non-unit loop dim is expected");
843+
assert(
844+
(linalgOp.hasDynamicShape() ||
845+
llvm::count_if(loopRanges, [](int64_t dim) { return dim != 1; }) == 1) &&
846+
"For statically shaped Linalg Ops, only one "
847+
"non-unit loop dim is expected");
848848

849849
size_t idx = loopRanges.size() - 1;
850850
for (; idx >= 0; idx--)

mlir/lib/Dialect/Tensor/Utils/Utils.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ PadOp mlir::tensor::createPadHighOp(RankedTensorType resType, Value source,
2727
OpBuilder &b,
2828
SmallVector<Value> dynOutDims) {
2929

30-
assert((resType.getNumDynamicDims() == dynOutDims.size()) ||
31-
dynOutDims.empty() &&
32-
"Either none or all output dynamic dims must be specified!");
30+
assert(((resType.getNumDynamicDims() == dynOutDims.size()) ||
31+
dynOutDims.empty()) &&
32+
"Either none or all output dynamic dims must be specified!");
3333

3434
// Init "low" and "high" padding values ("low" is kept as is, "high" is
3535
// computed below).

0 commit comments

Comments
 (0)