Skip to content

Commit 7a98071

Browse files
dtzSiFivedtzWill
andauthored
[mlir] Verifier: steal bit to track seen instead of set. (llvm#102626)
Tracking a set containing every block and operation visited can become very expensive and is unnecessary. Co-authored-by: Will Dietz <[email protected]>
1 parent 52126dc commit 7a98071

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

mlir/lib/IR/Verifier.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "mlir/IR/RegionKindInterface.h"
3333
#include "mlir/IR/Threading.h"
3434
#include "llvm/ADT/DenseMapInfoVariant.h"
35+
#include "llvm/ADT/PointerIntPair.h"
3536
#include "llvm/ADT/StringMap.h"
3637
#include "llvm/Support/FormatVariadic.h"
3738
#include "llvm/Support/PrettyStackTrace.h"
@@ -55,6 +56,7 @@ class OperationVerifier {
5556

5657
private:
5758
using WorkItem = llvm::PointerUnion<Operation *, Block *>;
59+
using WorkItemEntry = llvm::PointerIntPair<WorkItem, 1, bool>;
5860

5961
/// This verifier uses a DFS of the tree of operations/blocks. The method
6062
/// verifyOnEntrance is invoked when we visit a node for the first time, i.e.
@@ -267,35 +269,38 @@ LogicalResult OperationVerifier::verifyOnExit(Operation &op) {
267269
/// Such ops are collected separately and verified inside
268270
/// verifyBlockPostChildren.
269271
LogicalResult OperationVerifier::verifyOperation(Operation &op) {
270-
SmallVector<WorkItem> worklist{{&op}};
271-
SmallPtrSet<WorkItem, 8> seen;
272+
SmallVector<WorkItemEntry> worklist{{&op, false}};
272273
while (!worklist.empty()) {
273-
WorkItem top = worklist.back();
274+
WorkItemEntry &top = worklist.back();
274275

275276
auto visit = [](auto &&visitor, WorkItem w) {
276277
if (w.is<Operation *>())
277278
return visitor(w.get<Operation *>());
278279
return visitor(w.get<Block *>());
279280
};
280281

281-
const bool isExit = !seen.insert(top).second;
282+
const bool isExit = top.getInt();
283+
top.setInt(true);
284+
auto item = top.getPointer();
285+
282286
// 2nd visit of this work item ("exit").
283287
if (isExit) {
284-
worklist.pop_back();
285-
if (failed(visit(
286-
[this](auto *workItem) { return verifyOnExit(*workItem); }, top)))
288+
if (failed(
289+
visit([this](auto *workItem) { return verifyOnExit(*workItem); },
290+
item)))
287291
return failure();
292+
worklist.pop_back();
288293
continue;
289294
}
290295

291296
// 1st visit of this work item ("entrance").
292297
if (failed(visit(
293298
[this](auto *workItem) { return verifyOnEntrance(*workItem); },
294-
top)))
299+
item)))
295300
return failure();
296301

297-
if (top.is<Block *>()) {
298-
Block &currentBlock = *top.get<Block *>();
302+
if (item.is<Block *>()) {
303+
Block &currentBlock = *item.get<Block *>();
299304
// Skip "isolated from above operations".
300305
for (Operation &o : llvm::reverse(currentBlock)) {
301306
if (o.getNumRegions() == 0 ||
@@ -305,7 +310,7 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
305310
continue;
306311
}
307312

308-
Operation &currentOp = *top.get<Operation *>();
313+
Operation &currentOp = *item.get<Operation *>();
309314
if (verifyRecursively)
310315
for (Region &region : llvm::reverse(currentOp.getRegions()))
311316
for (Block &block : llvm::reverse(region))

0 commit comments

Comments
 (0)