Skip to content

Commit c10077d

Browse files
noclownsaokblast
authored andcommitted
[MLIR] Fix use-after-move in debug logging (llvm#165208)
1. In `Transforms.cpp` the debug macro is accessing a SmallVector variable that has been moved-from and reset. Fixed by reordering code for the move-from to happen last. 2. `IterationGraphSorter` Refine the previous use-after-move fix for style/readability by renaming the private constructor args to resolve naming ambiguity with the class members. Testing: `ninja check-mlir`
1 parent 69c06d0 commit c10077d

File tree

3 files changed

+13
-11
lines changed

3 files changed

+13
-11
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,13 +495,14 @@ FailureOr<PackResult> linalg::pack(RewriterBase &rewriter,
495495
if (failed(maybePackedDimForEachOperand))
496496
return failure();
497497
packedOperandsDims.packedDimForEachOperand = *maybePackedDimForEachOperand;
498-
listOfPackedOperandsDim.pushBack(std::move(packedOperandsDims));
499498

500499
LDBG() << "++++ After pack size #" << i << ": " << packedSizes[i];
501500
LDBG() << "maps: " << llvm::interleaved(indexingMaps);
502501
LDBG() << "iterators: " << llvm::interleaved(iteratorTypes);
503502
LDBG() << "packedDimForEachOperand: "
504503
<< llvm::interleaved(packedOperandsDims.packedDimForEachOperand);
504+
505+
listOfPackedOperandsDim.pushBack(std::move(packedOperandsDims));
505506
}
506507

507508
// Step 2. Propagate packing to all LinalgOp operands.

mlir/lib/Dialect/SparseTensor/Transforms/Utils/IterationGraphSorter.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,19 +152,20 @@ IterationGraphSorter IterationGraphSorter::fromGenericOp(
152152
}
153153

154154
IterationGraphSorter::IterationGraphSorter(
155-
SmallVector<Value> &&ins, SmallVector<AffineMap> &&loop2InsLvl, Value out,
156-
AffineMap loop2OutLvl, SmallVector<utils::IteratorType> &&iterTypes,
155+
SmallVector<Value> &&insArg, SmallVector<AffineMap> &&loop2InsLvlArg,
156+
Value out, AffineMap loop2OutLvl,
157+
SmallVector<utils::IteratorType> &&iterTypesArg,
157158
sparse_tensor::LoopOrderingStrategy strategy)
158-
: ins(std::move(ins)), loop2InsLvl(std::move(loop2InsLvl)), out(out),
159-
loop2OutLvl(loop2OutLvl), iterTypes(std::move(iterTypes)),
159+
: ins(std::move(insArg)), loop2InsLvl(std::move(loop2InsLvlArg)), out(out),
160+
loop2OutLvl(loop2OutLvl), iterTypes(std::move(iterTypesArg)),
160161
strategy(strategy) {
161162
// One map per tensor.
162-
assert(this->loop2InsLvl.size() == this->ins.size());
163+
assert(loop2InsLvl.size() == ins.size());
163164
// All the affine maps have the same number of dimensions (loops).
164165
assert(llvm::all_equal(llvm::map_range(
165-
this->loop2InsLvl, [](AffineMap m) { return m.getNumDims(); })));
166+
loop2InsLvl, [](AffineMap m) { return m.getNumDims(); })));
166167
// The number of results of the map should match the rank of the tensor.
167-
assert(llvm::all_of(llvm::zip(this->loop2InsLvl, this->ins), [](auto mvPair) {
168+
assert(llvm::all_of(llvm::zip(loop2InsLvl, ins), [](auto mvPair) {
168169
auto [m, v] = mvPair;
169170

170171
// For ranked types the rank must match.

mlir/lib/Dialect/SparseTensor/Transforms/Utils/IterationGraphSorter.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ class IterationGraphSorter {
5959

6060
private:
6161
// Private constructor.
62-
IterationGraphSorter(SmallVector<Value> &&ins,
63-
SmallVector<AffineMap> &&loop2InsLvl, Value out,
62+
IterationGraphSorter(SmallVector<Value> &&insArg,
63+
SmallVector<AffineMap> &&loop2InsLvlArg, Value out,
6464
AffineMap loop2OutLvl,
65-
SmallVector<utils::IteratorType> &&iterTypes,
65+
SmallVector<utils::IteratorType> &&iterTypesArg,
6666
sparse_tensor::LoopOrderingStrategy strategy =
6767
sparse_tensor::LoopOrderingStrategy::kDefault);
6868

0 commit comments

Comments
 (0)