Skip to content

Commit ed57433

Browse files
[mlir][Transforms] Fix compile time regression in dialect conversion
The dialect conversion does not directly erase ops that are replaced/erased with a rewriter. Instead, the op stays in place and is erased at the end if the dialect conversion succeeds. However, ops that were replaced/erased are ignored from that point on. #81757 introduced a compile time regression that made the check whether an op is ignored or not more expensive. Whether an op is ignored or not is queried many times throughout a dialect conversion, so the check must be fast. After this change, replaced ops are stored in the `ignoredOps` set. This also simplifies the dialect conversion a bit.
1 parent 2730a5c commit ed57433

File tree

1 file changed

+4
-9
lines changed

1 file changed

+4
-9
lines changed

mlir/lib/Transforms/Utils/DialectConversion.cpp

+4-9
Original file line numberDiff line numberDiff line change
@@ -1229,9 +1229,8 @@ LogicalResult ConversionPatternRewriterImpl::remapValues(
12291229
}
12301230

12311231
bool ConversionPatternRewriterImpl::isOpIgnored(Operation *op) const {
1232-
// Check to see if this operation was replaced or its parent ignored.
1233-
return ignoredOps.count(op->getParentOp()) ||
1234-
hasRewrite<ReplaceOperationRewrite>(rewrites, op);
1232+
// Check to see if this operation or the parent operation is ignored.
1233+
return ignoredOps.count(op->getParentOp()) || ignoredOps.count(op);
12351234
}
12361235

12371236
void ConversionPatternRewriterImpl::markNestedOpsIgnored(Operation *op) {
@@ -1480,12 +1479,7 @@ void ConversionPatternRewriterImpl::notifyOperationInserted(
14801479
void ConversionPatternRewriterImpl::notifyOpReplaced(Operation *op,
14811480
ValueRange newValues) {
14821481
assert(newValues.size() == op->getNumResults());
1483-
#ifndef NDEBUG
1484-
for (auto &rewrite : rewrites)
1485-
if (auto *opReplacement = dyn_cast<ReplaceOperationRewrite>(rewrite.get()))
1486-
assert(opReplacement->getOperation() != op &&
1487-
"operation was already replaced");
1488-
#endif // NDEBUG
1482+
assert(!ignoredOps.contains(op) && "operation was already replaced");
14891483

14901484
// Track if any of the results changed, e.g. erased and replaced with null.
14911485
bool resultChanged = false;
@@ -1506,6 +1500,7 @@ void ConversionPatternRewriterImpl::notifyOpReplaced(Operation *op,
15061500

15071501
// Mark this operation as recursively ignored so that we don't need to
15081502
// convert any nested operations.
1503+
ignoredOps.insert(op);
15091504
markNestedOpsIgnored(op);
15101505
}
15111506

0 commit comments

Comments
 (0)