Skip to content

Commit 44f6e86

Browse files
committed
[mlir] teach expensive-checks transform mode about empty handle
The transform dialect interpreter features the expensive-checks mode that acts as an embedded sanitizer to track use-after-consume of transform handles. Its logic is based on the relations between payload operations, which made it silently ignore empty handles that are consumed. Also catch and report this case because the remaining code may hit an assertion on attempting to access a consumed handle (that is removed from the mapping). Reviewed By: nicolasvasilache Differential Revision: https://reviews.llvm.org/D151560
1 parent e31f994 commit 44f6e86

File tree

4 files changed

+50
-2
lines changed

4 files changed

+50
-2
lines changed

mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ void transform::TransformState::recordOpHandleInvalidationOne(
443443
llvm::interleaveComma(potentialAncestors, DBGS() << "--ancestors: ",
444444
[](Operation *op) { llvm::dbgs() << *op; });
445445
llvm::dbgs() << "\n");
446+
447+
Operation *owner = consumingHandle.getOwner();
448+
unsigned operandNo = consumingHandle.getOperandNumber();
446449
for (Operation *ancestor : potentialAncestors) {
447450
// clang-format off
448451
DEBUG_WITH_TYPE(DEBUG_TYPE_FULL,
@@ -462,8 +465,6 @@ void transform::TransformState::recordOpHandleInvalidationOne(
462465
// deleted before the lambda gets called.
463466
Location ancestorLoc = ancestor->getLoc();
464467
Location opLoc = payloadOp->getLoc();
465-
Operation *owner = consumingHandle.getOwner();
466-
unsigned operandNo = consumingHandle.getOperandNumber();
467468
std::optional<Location> throughValueLoc =
468469
throughValue ? std::make_optional(throughValue.getLoc()) : std::nullopt;
469470
invalidatedHandles[otherHandle] = [ancestorLoc, opLoc, owner, operandNo,
@@ -551,6 +552,27 @@ void transform::TransformState::recordValueHandleInvalidationByOpHandleOne(
551552
void transform::TransformState::recordOpHandleInvalidation(
552553
OpOperand &handle, ArrayRef<Operation *> potentialAncestors,
553554
Value throughValue) {
555+
556+
if (potentialAncestors.empty()) {
557+
DEBUG_WITH_TYPE(DEBUG_TYPE_FULL, {
558+
(DBGS() << "----recording invalidation for empty handle: " << handle.get()
559+
<< "\n");
560+
});
561+
562+
Operation *owner = handle.getOwner();
563+
unsigned operandNo = handle.getOperandNumber();
564+
invalidatedHandles[handle.get()] = [owner, operandNo](Location currentLoc) {
565+
InFlightDiagnostic diag = emitError(currentLoc)
566+
<< "op uses a handle associated with empty "
567+
"payload and invalidated by a "
568+
"previously executed transform op";
569+
diag.attachNote(owner->getLoc())
570+
<< "invalidated by this transform op that consumes its operand #"
571+
<< operandNo;
572+
};
573+
return;
574+
}
575+
554576
// Iterate over the mapping and invalidate aliasing handles. This is quite
555577
// expensive and only necessary for error reporting in case of transform
556578
// dialect misuse with dangling handles. Iteration over the handles is based

mlir/test/Dialect/Transform/expensive-checks.mlir

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,14 @@ transform.sequence failures(propagate) {
331331
test_consume_operand %3 : !transform.any_value
332332
test_consume_operand %2 : !transform.any_op
333333
}
334+
335+
// -----
336+
337+
transform.sequence failures(propagate) {
338+
^bb0(%arg0: !transform.any_op):
339+
%0 = transform.test_produce_empty_payload : !transform.any_op
340+
// expected-note @below {{invalidated by this transform op that consumes its operand #0}}
341+
transform.test_consume_operand %0 : !transform.any_op
342+
// expected-error @below {{uses a handle associated with empty payload and invalidated by a previously executed transform op}}
343+
transform.test_print_remark_at_operand %0, "remark" : !transform.any_op
344+
}

mlir/test/lib/Dialect/Transform/TestTransformDialectExtension.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,12 @@ DiagnosedSilenceableFailure mlir::test::TestProduceNullPayloadOp::apply(
627627
return DiagnosedSilenceableFailure::success();
628628
}
629629

630+
DiagnosedSilenceableFailure mlir::test::TestProduceEmptyPayloadOp::apply(
631+
transform::TransformResults &results, transform::TransformState &state) {
632+
results.set(cast<OpResult>(getOut()), {});
633+
return DiagnosedSilenceableFailure::success();
634+
}
635+
630636
void mlir::test::TestProduceNullParamOp::getEffects(
631637
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
632638
transform::producesHandle(getOut(), effects);

mlir/test/lib/Dialect/Transform/TestTransformDialectExtension.td

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,15 @@ def TestProduceNullPayloadOp
427427
let cppNamespace = "::mlir::test";
428428
}
429429

430+
def TestProduceEmptyPayloadOp
431+
: Op<Transform_Dialect, "test_produce_empty_payload",
432+
[DeclareOpInterfaceMethods<TransformOpInterface>,
433+
MemoryEffectsOpInterface, FunctionalStyleTransformOpTrait]> {
434+
let results = (outs TransformHandleTypeInterface:$out);
435+
let assemblyFormat = "attr-dict `:` type($out)";
436+
let cppNamespace = "::mlir::test";
437+
}
438+
430439
def TestProduceNullParamOp
431440
: Op<Transform_Dialect, "test_produce_null_param",
432441
[DeclareOpInterfaceMethods<MemoryEffectsOpInterface>,

0 commit comments

Comments
 (0)