Skip to content

Commit a327b2d

Browse files
authored
[Hoisting] Fix the double-free issue in HoistIntoGlobalsPass::cleanupDeadOp. (#21699)
The `ConstExprAnalysis::populateConstExprOperations()` just returns the const-expr ops in arbitrary order. We might try to delete the child op after parent op's erase() call if we just iterate that arbitrary order list for dead ops . Then, we will have double-free issue. Thus, we skip the original `cleanupDeadOps()` call at the end. The dead ops could be removed in later pass. The `hoist_into_globals.mlir` needs to updated for the dead ops. Signed-off-by: Jerry Shih <[email protected]>
1 parent 9303360 commit a327b2d

File tree

1 file changed

+18
-22
lines changed

1 file changed

+18
-22
lines changed

compiler/src/iree/compiler/Dialect/Util/Transforms/HoistIntoGlobals.cpp

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "mlir/IR/Builders.h"
1616
#include "mlir/IR/BuiltinTypes.h"
1717
#include "mlir/IR/IRMapping.h"
18+
#include "mlir/IR/Iterators.h"
1819
#include "mlir/IR/SymbolTable.h"
1920

2021
#define DEBUG_TYPE "iree-constexpr"
@@ -328,29 +329,24 @@ class HoistIntoGlobalsPass
328329

329330
// Since we are mutating the const-expr ops, the ConstExprAnalysis will no
330331
// longer be valid after this point.
331-
SmallVector<Operation *> worklist;
332-
worklist.reserve(allOps.size());
333-
bool madeChanges = true;
334-
while (madeChanges) {
335-
madeChanges = false;
336-
337-
// Prepare worklist.
338-
worklist.clear();
339-
worklist.append(allOps.begin(), allOps.end());
340-
341-
for (Operation *checkOp : worklist) {
342-
if (checkOp->use_empty()) {
343-
// Bingo.
344-
LLVM_DEBUG({
345-
llvm::dbgs() << "[HoistIntoGlobals] erase dead op: ";
346-
checkOp->print(llvm::dbgs(), constExprs.getAsmState());
347-
llvm::dbgs() << "\n";
332+
for (auto funcOp : getOperation().getOps<FunctionOpInterface>()) {
333+
// Ignore initializers.
334+
if (isa<IREE::Util::InitializerOpInterface>(funcOp.getOperation()))
335+
continue;
336+
funcOp.walk<WalkOrder::PostOrder, ReverseIterator>(
337+
[&](Operation *iterOp) {
338+
if (allOps.contains(iterOp) && iterOp->use_empty()) {
339+
// Bingo.
340+
LLVM_DEBUG({
341+
llvm::dbgs() << "[HoistIntoGlobals] erase dead op: ";
342+
iterOp->print(llvm::dbgs(), constExprs.getAsmState());
343+
llvm::dbgs() << "\n";
344+
});
345+
allOps.erase(iterOp);
346+
iterOp->erase();
347+
}
348+
return WalkResult::advance();
348349
});
349-
madeChanges = true;
350-
allOps.erase(checkOp);
351-
checkOp->erase();
352-
}
353-
}
354350
}
355351
}
356352

0 commit comments

Comments
 (0)