Skip to content

[mlir][transform] Don't modify the target in interpreter when loading library. #67686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ LogicalResult interpreterBaseRunOnOperationImpl(
/// will be interpreted.
/// - transformLibraryFileName: if non-empty, the name of the file containing
/// definitions of external symbols referenced in the transform script.
/// These definitions will be used to replace declarations.
/// These definitions will be used to resolve declarations.
/// - debugPayloadRootTag: if non-empty, the value of the attribute named
/// `kTransformDialectTagAttrName` indicating the single op that is
/// considered the payload root of the transform interpreter; otherwise, the
Expand All @@ -85,7 +85,7 @@ LogicalResult interpreterBaseRunOnOperationImpl(
/// as template arguments. They are *not* expected to to implement `initialize`
/// or `runOnOperation`. They *are* expected to call the copy constructor of
/// this class in their copy constructors, short of which the file-based
/// transform dialect script injection facility will become nonoperational.
/// transform dialect script resolution facility will become non-operational.
///
/// Concrete passes may implement the `runBeforeInterpreter` and
/// `runAfterInterpreter` to customize the behavior of the pass.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,24 @@ LogicalResult transform::detail::interpreterBaseRunOnOperationImpl(
// transform is embedded in the payload IR. If debugTransformRootTag was
// passed, then we are in user-specified selection of the transforming IR.
// This corresponds to REPL debug mode.
Operation *transformContainer =
hasSharedTransformModule ? sharedTransformModule->get() : target;

OwningOpRef<Operation *> transformContainerClone;
Operation *transformContainer;
if (hasTransformLibraryModule) {
// If we have a library module, then the transform script is embedded in the
// target, which we don't want to modify when loading the library. We thus
// clone the target and use that as transform container.
assert(!hasSharedTransformModule);
transformContainerClone = target->clone();
transformContainer = transformContainerClone.get();
} else {
// If we have a shared library, which is private to us, we can modify it
// when loading the library, so we use that. Otherwise, we don't have any
// library to load, so we can use the target and won't modify it.
transformContainer =
hasSharedTransformModule ? sharedTransformModule->get() : target;
}

Operation *transformRoot =
debugTransformRootTag.empty()
? findTopLevelTransform(transformContainer,
Expand All @@ -436,7 +452,7 @@ LogicalResult transform::detail::interpreterBaseRunOnOperationImpl(
// concurrent execution (normally, the error shouldn't be triggered unless the
// transform IR modifies itself in a pass, which is also forbidden elsewhere).
if (hasTransformLibraryModule) {
if (!target->isProperAncestor(transformRoot)) {
if (!transformContainer->isProperAncestor(transformRoot)) {
InFlightDiagnostic diag =
transformRoot->emitError()
<< "cannot inject transform definitions next to pass anchor op";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-file-name=%p/test-interpreter-external-symbol-def.mlir})" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s

// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-file-name=%p/test-interpreter-external-symbol-def.mlir}, test-transform-dialect-interpreter)" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s

// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-file-name=%p/test-interpreter-external-symbol-def.mlir}, test-transform-dialect-interpreter{transform-library-file-name=%p/test-interpreter-external-symbol-def.mlir})" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s

// The definition of the @foo named sequence is provided in another file. It
// will be included because of the pass option. Repeated application of the
// same pass, with or without the library option, should not be a problem.
// will be available because of the pass option but not included in the output.
// Repeated application of the same pass works, but only if the library is
// provided in both.
// Note that the same diagnostic produced twice at the same location only
// needs to be matched once.

// expected-remark @below {{message}}
// expected-remark @below {{unannotated}}
module attributes {transform.with_named_sequence} {
// CHECK: transform.named_sequence @foo
// CHECK: test_print_remark_at_operand %{{.*}}, "message"
// CHECK: transform.named_sequence private @foo
// CHECK-NOT: test_print_remark_at_operand
transform.named_sequence private @foo(!transform.any_op {transform.readonly})

// CHECK: transform.named_sequence @unannotated
// CHECK: test_print_remark_at_operand %{{.*}}, "unannotated"
// CHECK: transform.named_sequence private @unannotated
// CHECK-NOT: test_print_remark_at_operand
transform.named_sequence private @unannotated(!transform.any_op {transform.readonly})

transform.sequence failures(propagate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ class TestTransformDialectInterpreterPass
Option<std::string> transformLibraryFileName{
*this, "transform-library-file-name", llvm::cl::init(""),
llvm::cl::desc(
"Optional name of the file containing transform dialect symbol "
"definitions to be injected into the transform module.")};
"Optional name of the file providing transform dialect definitions "
"from which declarations in the transform module can be resolved.")};

Option<bool> testModuleGeneration{
*this, "test-module-generation", llvm::cl::init(false),
Expand Down