[Mlir-commits] [mlir] [MLIR][Transform] Allow ApplyRegisteredPassOp to take options as a param (PR #142683)
Rolf Morel
llvmlistbot at llvm.org
Fri Jun 6 03:10:45 PDT 2025
https://github.com/rolfmorel updated https://github.com/llvm/llvm-project/pull/142683
>From 897730627e603729dfaee5aa67eb6db7f488074f Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Tue, 3 Jun 2025 15:08:55 -0700
Subject: [PATCH 1/4] [MLIR][Transform] Allow ApplyRegisteredPassOp to take
options as a param
Makes it possible to pass around the options to a pass inside a schedule.
The refactoring also makes it so that the pass manager and pass are only
constructed once per apply of the transform op versus for each target
payload given to the op.
---
.../mlir/Dialect/Transform/IR/TransformOps.td | 25 ++--
.../lib/Dialect/Transform/IR/TransformOps.cpp | 117 +++++++++++++++---
.../Transform/test-pass-application.mlir | 53 +++++++-
3 files changed, 160 insertions(+), 35 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td b/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
index e4eb67c8e14ce..b042f5e436185 100644
--- a/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
+++ b/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
@@ -399,15 +399,15 @@ def ApplyLoopInvariantCodeMotionOp : TransformDialectOp<"apply_licm",
}
def ApplyRegisteredPassOp : TransformDialectOp<"apply_registered_pass",
- [TransformOpInterface, TransformEachOpTrait,
- FunctionalStyleTransformOpTrait, MemoryEffectsOpInterface]> {
+ [DeclareOpInterfaceMethods<TransformOpInterface>,
+ DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
let summary = "Applies the specified registered pass or pass pipeline";
let description = [{
This transform applies the specified pass or pass pipeline to the targeted
ops. The name of the pass/pipeline is specified as a string attribute, as
set during pass/pipeline registration. Optionally, pass options may be
- specified as a string attribute. The pass options syntax is identical to the
- one used with "mlir-opt".
+ specified as a string attribute with the option to pass the attribute as a
+ param. The pass options syntax is identical to the one used with "mlir-opt".
This op first looks for a pass pipeline with the specified name. If no such
pipeline exists, it looks for a pass with the specified name. If no such
@@ -420,20 +420,15 @@ def ApplyRegisteredPassOp : TransformDialectOp<"apply_registered_pass",
of targeted ops.
}];
- let arguments = (ins TransformHandleTypeInterface:$target,
+ let arguments = (ins Optional<TransformParamTypeInterface>:$dynamic_options,
+ TransformHandleTypeInterface:$target,
StrAttr:$pass_name,
- DefaultValuedAttr<StrAttr, "\"\"">:$options);
+ DefaultValuedAttr<StrAttr, "\"\"">:$static_options);
let results = (outs TransformHandleTypeInterface:$result);
let assemblyFormat = [{
- $pass_name `to` $target attr-dict `:` functional-type(operands, results)
- }];
-
- let extraClassDeclaration = [{
- ::mlir::DiagnosedSilenceableFailure applyToOne(
- ::mlir::transform::TransformRewriter &rewriter,
- ::mlir::Operation *target,
- ::mlir::transform::ApplyToEachResultList &results,
- ::mlir::transform::TransformState &state);
+ $pass_name (`with` `options` `=`
+ custom<ApplyRegisteredPassOptions>($dynamic_options, $static_options)^)?
+ `to` $target attr-dict `:` functional-type(operands, results)
}];
}
diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
index 673743f22249a..536c3e14fe5c0 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
@@ -53,6 +53,13 @@
using namespace mlir;
+static ParseResult parseApplyRegisteredPassOptions(
+ OpAsmParser &parser,
+ std::optional<OpAsmParser::UnresolvedOperand> &dynamicOptions,
+ StringAttr &staticOptions);
+static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
+ Operation *op, Value dynamicOptions,
+ StringAttr staticOptions);
static ParseResult parseSequenceOpOperands(
OpAsmParser &parser, std::optional<OpAsmParser::UnresolvedOperand> &root,
Type &rootType,
@@ -766,17 +773,38 @@ void transform::ApplyLoopInvariantCodeMotionOp::getEffects(
// ApplyRegisteredPassOp
//===----------------------------------------------------------------------===//
-DiagnosedSilenceableFailure transform::ApplyRegisteredPassOp::applyToOne(
- transform::TransformRewriter &rewriter, Operation *target,
- ApplyToEachResultList &results, transform::TransformState &state) {
- // Make sure that this transform is not applied to itself. Modifying the
- // transform IR while it is being interpreted is generally dangerous. Even
- // more so when applying passes because they may perform a wide range of IR
- // modifications.
- DiagnosedSilenceableFailure payloadCheck =
- ensurePayloadIsSeparateFromTransform(*this, target);
- if (!payloadCheck.succeeded())
- return payloadCheck;
+void transform::ApplyRegisteredPassOp::getEffects(
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ consumesHandle(getTargetMutable(), effects);
+ onlyReadsHandle(getDynamicOptionsMutable(), effects);
+ producesHandle(getOperation()->getOpResults(), effects);
+ modifiesPayload(effects);
+}
+
+DiagnosedSilenceableFailure
+transform::ApplyRegisteredPassOp::apply(transform::TransformRewriter &rewriter,
+ transform::TransformResults &results,
+ transform::TransformState &state) {
+ // Check whether pass options are specified, either as a dynamic param or
+ // a static attribute. In either case, options are passed as a single string.
+ StringRef options;
+ if (auto dynamicOptions = getDynamicOptions()) {
+ ArrayRef<Attribute> dynamicOptionsParam = state.getParams(dynamicOptions);
+ if (dynamicOptionsParam.size() != 1) {
+ return emitSilenceableError()
+ << "options passed as a param must be a single value, got "
+ << dynamicOptionsParam.size();
+ }
+ if (auto optionsStrAttr = dyn_cast<StringAttr>(dynamicOptionsParam[0])) {
+ options = optionsStrAttr.getValue();
+ } else {
+ return emitSilenceableError()
+ << "options passed as a param must be a string, got "
+ << dynamicOptionsParam[0];
+ }
+ } else {
+ options = getStaticOptions();
+ }
// Get pass or pass pipeline from registry.
const PassRegistryEntry *info = PassPipelineInfo::lookup(getPassName());
@@ -786,9 +814,9 @@ DiagnosedSilenceableFailure transform::ApplyRegisteredPassOp::applyToOne(
return emitDefiniteFailure()
<< "unknown pass or pass pipeline: " << getPassName();
- // Create pass manager and run the pass or pass pipeline.
+ // Create pass manager and add the pass or pass pipeline.
PassManager pm(getContext());
- if (failed(info->addToPipeline(pm, getOptions(), [&](const Twine &msg) {
+ if (failed(info->addToPipeline(pm, options, [&](const Twine &msg) {
emitError(msg);
return failure();
}))) {
@@ -796,16 +824,69 @@ DiagnosedSilenceableFailure transform::ApplyRegisteredPassOp::applyToOne(
<< "failed to add pass or pass pipeline to pipeline: "
<< getPassName();
}
- if (failed(pm.run(target))) {
- auto diag = emitSilenceableError() << "pass pipeline failed";
- diag.attachNote(target->getLoc()) << "target op";
- return diag;
+
+ auto targets = SmallVector<Operation *>(state.getPayloadOps(getTarget()));
+ for (Operation *target : targets) {
+ // Make sure that this transform is not applied to itself. Modifying the
+ // transform IR while it is being interpreted is generally dangerous. Even
+ // more so when applying passes because they may perform a wide range of IR
+ // modifications.
+ DiagnosedSilenceableFailure payloadCheck =
+ ensurePayloadIsSeparateFromTransform(*this, target);
+ if (!payloadCheck.succeeded())
+ return payloadCheck;
+
+ // Run the pass or pass pipeline on the current target operation.
+ if (failed(pm.run(target))) {
+ auto diag = emitSilenceableError() << "pass pipeline failed";
+ diag.attachNote(target->getLoc()) << "target op";
+ return diag;
+ }
}
- results.push_back(target);
+ // The applied pass will have directly modified the payload IR(s).
+ results.set(llvm::cast<OpResult>(getResult()), targets);
return DiagnosedSilenceableFailure::success();
}
+static ParseResult parseApplyRegisteredPassOptions(
+ OpAsmParser &parser,
+ std::optional<OpAsmParser::UnresolvedOperand> &dynamicOptions,
+ StringAttr &staticOptions) {
+ dynamicOptions = std::nullopt;
+ OpAsmParser::UnresolvedOperand dynamicOptionsOperand;
+ OptionalParseResult hasDynamicOptions =
+ parser.parseOptionalOperand(dynamicOptionsOperand);
+
+ if (hasDynamicOptions.has_value()) {
+ if (failed(hasDynamicOptions.value()))
+ return failure();
+
+ dynamicOptions = dynamicOptionsOperand;
+ return success();
+ }
+
+ OptionalParseResult hasStaticOptions =
+ parser.parseOptionalAttribute(staticOptions);
+ if (hasStaticOptions.has_value()) {
+ if (failed(hasStaticOptions.value()))
+ return failure();
+ return success();
+ }
+
+ return success();
+}
+
+static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
+ Operation *op, Value dynamicOptions,
+ StringAttr staticOptions) {
+ if (dynamicOptions) {
+ printer.printOperand(dynamicOptions);
+ } else if (!staticOptions.getValue().empty()) {
+ printer.printAttribute(staticOptions);
+ }
+}
+
//===----------------------------------------------------------------------===//
// CastOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/Transform/test-pass-application.mlir b/mlir/test/Dialect/Transform/test-pass-application.mlir
index 3a40b462b8270..e8e0f63b28096 100644
--- a/mlir/test/Dialect/Transform/test-pass-application.mlir
+++ b/mlir/test/Dialect/Transform/test-pass-application.mlir
@@ -79,7 +79,7 @@ module attributes {transform.with_named_sequence} {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
// expected-error @below {{failed to add pass or pass pipeline to pipeline: canonicalize}}
// expected-error @below {{<Pass-Options-Parser>: no such option invalid-option}}
- transform.apply_registered_pass "canonicalize" to %1 {options = "invalid-option=1"} : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize" with options = "invalid-option=1" to %1 : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -94,7 +94,56 @@ func.func @valid_pass_option() {
module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
- transform.apply_registered_pass "canonicalize" to %1 {options = "top-down=false"} : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize" with options = "top-down=false" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
+// -----
+
+// CHECK-LABEL: func @valid_dynamic_pass_option()
+func.func @valid_dynamic_pass_option() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %pass_options = transform.param.constant "top-down=false" -> !transform.any_param
+ transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+// -----
+
+func.func @invalid_pass_option_param() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %pass_options = transform.param.constant 42 -> !transform.any_param
+ // expected-error @below {{options passed as a param must be a string, got 42}}
+ transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize" with options = "invalid-option=1" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
+// -----
+
+func.func @too_many_pass_option_params() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %x = transform.param.constant "x" -> !transform.any_param
+ %pass_options = transform.merge_handles %x, %x : !transform.any_param
+ // expected-error @below {{options passed as a param must be a single value, got 2}}
+ transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
>From d57d60aeb034d54d3f0e3a285fe7e1d3a2bebd77 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Thu, 5 Jun 2025 05:39:02 -0700
Subject: [PATCH 2/4] Allow passing in multiple separate option strings and/or
params
---
.../mlir/Dialect/Transform/IR/TransformOps.td | 16 +-
.../lib/Dialect/Transform/IR/TransformOps.cpp | 153 ++++++++++++------
.../Transform/test-pass-application.mlir | 79 ++++++++-
3 files changed, 187 insertions(+), 61 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td b/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
index b042f5e436185..e864a65f8ceac 100644
--- a/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
+++ b/mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
@@ -406,8 +406,9 @@ def ApplyRegisteredPassOp : TransformDialectOp<"apply_registered_pass",
This transform applies the specified pass or pass pipeline to the targeted
ops. The name of the pass/pipeline is specified as a string attribute, as
set during pass/pipeline registration. Optionally, pass options may be
- specified as a string attribute with the option to pass the attribute as a
- param. The pass options syntax is identical to the one used with "mlir-opt".
+ specified as (space-separated) string attributes with the option to pass
+ these attributes via params. The pass options syntax is identical to the one
+ used with "mlir-opt".
This op first looks for a pass pipeline with the specified name. If no such
pipeline exists, it looks for a pass with the specified name. If no such
@@ -420,16 +421,17 @@ def ApplyRegisteredPassOp : TransformDialectOp<"apply_registered_pass",
of targeted ops.
}];
- let arguments = (ins Optional<TransformParamTypeInterface>:$dynamic_options,
- TransformHandleTypeInterface:$target,
- StrAttr:$pass_name,
- DefaultValuedAttr<StrAttr, "\"\"">:$static_options);
+ let arguments = (ins StrAttr:$pass_name,
+ DefaultValuedAttr<ArrayAttr, "{}">:$options,
+ Variadic<TransformParamTypeInterface>:$dynamic_options,
+ TransformHandleTypeInterface:$target);
let results = (outs TransformHandleTypeInterface:$result);
let assemblyFormat = [{
$pass_name (`with` `options` `=`
- custom<ApplyRegisteredPassOptions>($dynamic_options, $static_options)^)?
+ custom<ApplyRegisteredPassOptions>($options, $dynamic_options)^)?
`to` $target attr-dict `:` functional-type(operands, results)
}];
+ let hasVerifier = 1;
}
def CastOp : TransformDialectOp<"cast",
diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
index 536c3e14fe5c0..933d400932ec9 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
@@ -54,12 +54,11 @@
using namespace mlir;
static ParseResult parseApplyRegisteredPassOptions(
- OpAsmParser &parser,
- std::optional<OpAsmParser::UnresolvedOperand> &dynamicOptions,
- StringAttr &staticOptions);
+ OpAsmParser &parser, ArrayAttr &options,
+ SmallVectorImpl<OpAsmParser::UnresolvedOperand> &dynamicOptions);
static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
- Operation *op, Value dynamicOptions,
- StringAttr staticOptions);
+ Operation *op, ArrayAttr options,
+ ValueRange dynamicOptions);
static ParseResult parseSequenceOpOperands(
OpAsmParser &parser, std::optional<OpAsmParser::UnresolvedOperand> &root,
Type &rootType,
@@ -785,25 +784,40 @@ DiagnosedSilenceableFailure
transform::ApplyRegisteredPassOp::apply(transform::TransformRewriter &rewriter,
transform::TransformResults &results,
transform::TransformState &state) {
- // Check whether pass options are specified, either as a dynamic param or
- // a static attribute. In either case, options are passed as a single string.
- StringRef options;
- if (auto dynamicOptions = getDynamicOptions()) {
- ArrayRef<Attribute> dynamicOptionsParam = state.getParams(dynamicOptions);
- if (dynamicOptionsParam.size() != 1) {
- return emitSilenceableError()
- << "options passed as a param must be a single value, got "
- << dynamicOptionsParam.size();
- }
- if (auto optionsStrAttr = dyn_cast<StringAttr>(dynamicOptionsParam[0])) {
- options = optionsStrAttr.getValue();
+ // Obtain a single options-string from options passed statically as
+ // string attributes as well as "dynamically" through params.
+ std::string options;
+ OperandRange dynamicOptions = getDynamicOptions();
+ size_t dynamicOptionsIdx = 0;
+ for (auto [idx, optionAttr] : llvm::enumerate(getOptions())) {
+ if (idx > 0)
+ options += " "; // Interleave options seperator.
+
+ if (auto strAttr = dyn_cast<StringAttr>(optionAttr)) {
+ options += strAttr.getValue();
+ } else if (isa<UnitAttr>(optionAttr)) {
+ assert(dynamicOptionsIdx < dynamicOptions.size() &&
+ "number of dynamic option markers (UnitAttr) in options ArrayAttr "
+ "should be the same as the number of options passed as params");
+ ArrayRef<Attribute> dynamicOption =
+ state.getParams(dynamicOptions[dynamicOptionsIdx++]);
+ if (dynamicOption.size() != 1)
+ return emitSilenceableError() << "options passed as a param must have "
+ "a single value associated, param "
+ << dynamicOptionsIdx - 1 << " associates "
+ << dynamicOption.size();
+
+ if (auto dynamicOptionStr = dyn_cast<StringAttr>(dynamicOption[0])) {
+ options += dynamicOptionStr.getValue();
+ } else {
+ return emitSilenceableError()
+ << "options passed as a param must be a string, got "
+ << dynamicOption[0];
+ }
} else {
- return emitSilenceableError()
- << "options passed as a param must be a string, got "
- << dynamicOptionsParam[0];
+ assert(false &&
+ "expected options element to be either StringAttr or UnitAttr");
}
- } else {
- options = getStaticOptions();
}
// Get pass or pass pipeline from registry.
@@ -850,43 +864,88 @@ transform::ApplyRegisteredPassOp::apply(transform::TransformRewriter &rewriter,
}
static ParseResult parseApplyRegisteredPassOptions(
- OpAsmParser &parser,
- std::optional<OpAsmParser::UnresolvedOperand> &dynamicOptions,
- StringAttr &staticOptions) {
- dynamicOptions = std::nullopt;
- OpAsmParser::UnresolvedOperand dynamicOptionsOperand;
- OptionalParseResult hasDynamicOptions =
- parser.parseOptionalOperand(dynamicOptionsOperand);
-
- if (hasDynamicOptions.has_value()) {
- if (failed(hasDynamicOptions.value()))
- return failure();
+ OpAsmParser &parser, ArrayAttr &options,
+ SmallVectorImpl<OpAsmParser::UnresolvedOperand> &dynamicOptions) {
+ auto dynamicOptionMarker = UnitAttr::get(parser.getContext());
+ SmallVector<Attribute> optionsArray;
+
+ auto parseOperandOrString = [&]() -> OptionalParseResult {
+ OpAsmParser::UnresolvedOperand operand;
+ OptionalParseResult parsedOperand = parser.parseOptionalOperand(operand);
+ if (parsedOperand.has_value()) {
+ if (failed(parsedOperand.value()))
+ return failure();
- dynamicOptions = dynamicOptionsOperand;
- return success();
- }
+ dynamicOptions.push_back(operand);
+ optionsArray.push_back(
+ dynamicOptionMarker); // Placeholder for knowing where to
+ // inject the dynamic option-as-param.
+ return success();
+ }
- OptionalParseResult hasStaticOptions =
- parser.parseOptionalAttribute(staticOptions);
- if (hasStaticOptions.has_value()) {
- if (failed(hasStaticOptions.value()))
+ StringAttr stringAttr;
+ OptionalParseResult parsedStringAttr =
+ parser.parseOptionalAttribute(stringAttr);
+ if (parsedStringAttr.has_value()) {
+ if (failed(parsedStringAttr.value()))
+ return failure();
+ optionsArray.push_back(stringAttr);
+ return success();
+ }
+
+ return std::nullopt;
+ };
+
+ OptionalParseResult parsedOptionsElement = parseOperandOrString();
+ while (parsedOptionsElement.has_value()) {
+ if (failed(parsedOptionsElement.value()))
return failure();
- return success();
+ parsedOptionsElement = parseOperandOrString();
}
+ if (optionsArray.empty()) {
+ return parser.emitError(parser.getCurrentLocation())
+ << "expected at least one option (either a string or a param)";
+ }
+ options = parser.getBuilder().getArrayAttr(optionsArray);
return success();
}
static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
- Operation *op, Value dynamicOptions,
- StringAttr staticOptions) {
- if (dynamicOptions) {
- printer.printOperand(dynamicOptions);
- } else if (!staticOptions.getValue().empty()) {
- printer.printAttribute(staticOptions);
+ Operation *op, ArrayAttr options,
+ ValueRange dynamicOptions) {
+ size_t currentDynamicOptionIdx = 0;
+ for (Attribute optionAttr : options) {
+ if (currentDynamicOptionIdx > 0)
+ printer << " "; // Interleave options separator.
+
+ if (isa<UnitAttr>(optionAttr))
+ printer.printOperand(dynamicOptions[currentDynamicOptionIdx++]);
+ else if (auto strAttr = dyn_cast<StringAttr>(optionAttr))
+ printer.printAttribute(strAttr);
+ else
+ assert(false && "each option should be either a StringAttr or UnitAttr");
}
}
+LogicalResult transform::ApplyRegisteredPassOp::verify() {
+ size_t numUnitsInOptions = 0;
+ for (Attribute optionsElement : getOptions()) {
+ if (isa<UnitAttr>(optionsElement))
+ numUnitsInOptions++;
+ else if (!isa<StringAttr>(optionsElement))
+ return emitOpError() << "expected each option to be either a StringAttr "
+ << "or a UnitAttr, got " << optionsElement;
+ }
+
+ if (getDynamicOptions().size() != numUnitsInOptions)
+ return emitOpError()
+ << "expected the same number of options passed as params as "
+ << "UnitAttr elements in options ArrayAttr";
+
+ return success();
+}
+
//===----------------------------------------------------------------------===//
// CastOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/Transform/test-pass-application.mlir b/mlir/test/Dialect/Transform/test-pass-application.mlir
index e8e0f63b28096..61a5b947eb0d1 100644
--- a/mlir/test/Dialect/Transform/test-pass-application.mlir
+++ b/mlir/test/Dialect/Transform/test-pass-application.mlir
@@ -101,19 +101,84 @@ module attributes {transform.with_named_sequence} {
// -----
-// CHECK-LABEL: func @valid_dynamic_pass_option()
-func.func @valid_dynamic_pass_option() {
+// CHECK-LABEL: func @valid_pass_options()
+func.func @valid_pass_options() {
return
}
module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
- %pass_options = transform.param.constant "top-down=false" -> !transform.any_param
- transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
+ //transform.apply_registered_pass "canonicalize" with options = "top-down=false,max-iterations=10" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize" with options = "top-down=false test-convergence=true" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
+// -----
+
+// CHECK-LABEL: func @valid_pass_options_as_list()
+func.func @valid_pass_options_as_list() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize" with options = "top-down=false" "max-iterations=0" to %1 : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
+
+// -----
+
+// CHECK-LABEL: func @valid_dynamic_pass_options()
+func.func @valid_dynamic_pass_options() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
+ %max_rewrites = transform.param.constant "max-num-rewrites=1" -> !transform.any_param
+ %2 = transform.apply_registered_pass "canonicalize" with options = "top-down=false" %max_iter "test-convergence=true" %max_rewrites to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
+// -----
+
+func.func @invalid_dynamic_options_as_array() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
+ // expected-error @below {{expected at least one option (either a string or a param)}}
+ %2 = transform.apply_registered_pass "canonicalize" with options = ["top-down=false" %max_iter] to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
+// -----
+
+func.func @invalid_options_as_pairs() {
+ return
+}
+
+module attributes {transform.with_named_sequence} {
+ transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
+ %1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
+ %max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
+ // expected-error @below {{expected 'to'}}
+ %2 = transform.apply_registered_pass "canonicalize" with options = "top-down=" false to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.yield
+ }
+}
+
// -----
func.func @invalid_pass_option_param() {
@@ -126,7 +191,6 @@ module attributes {transform.with_named_sequence} {
%pass_options = transform.param.constant 42 -> !transform.any_param
// expected-error @below {{options passed as a param must be a string, got 42}}
transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
- transform.apply_registered_pass "canonicalize" with options = "invalid-option=1" to %1 : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -141,8 +205,9 @@ module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
%x = transform.param.constant "x" -> !transform.any_param
- %pass_options = transform.merge_handles %x, %x : !transform.any_param
- // expected-error @below {{options passed as a param must be a single value, got 2}}
+ %y = transform.param.constant "y" -> !transform.any_param
+ %pass_options = transform.merge_handles %x, %y : !transform.any_param
+ // expected-error @below {{options passed as a param must have a single value associated, param 0 associates 2}}
transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
>From 9529ea438120269e88b2ea506716e9aaeb912577 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Fri, 6 Jun 2025 01:54:27 -0700
Subject: [PATCH 3/4] Address reviews
---
.../lib/Dialect/Transform/IR/TransformOps.cpp | 8 ++--
.../Transform/test-pass-application.mlir | 41 +++++++++++++------
2 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
index 933d400932ec9..32197322980f6 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
@@ -815,8 +815,8 @@ transform::ApplyRegisteredPassOp::apply(transform::TransformRewriter &rewriter,
<< dynamicOption[0];
}
} else {
- assert(false &&
- "expected options element to be either StringAttr or UnitAttr");
+ llvm_unreachable(
+ "expected options element to be either StringAttr or UnitAttr");
}
}
@@ -915,8 +915,8 @@ static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
Operation *op, ArrayAttr options,
ValueRange dynamicOptions) {
size_t currentDynamicOptionIdx = 0;
- for (Attribute optionAttr : options) {
- if (currentDynamicOptionIdx > 0)
+ for (auto [idx, optionAttr] : llvm::enumerate(options)) {
+ if (idx > 0)
printer << " "; // Interleave options separator.
if (isa<UnitAttr>(optionAttr))
diff --git a/mlir/test/Dialect/Transform/test-pass-application.mlir b/mlir/test/Dialect/Transform/test-pass-application.mlir
index 61a5b947eb0d1..463fd98afa65c 100644
--- a/mlir/test/Dialect/Transform/test-pass-application.mlir
+++ b/mlir/test/Dialect/Transform/test-pass-application.mlir
@@ -79,7 +79,9 @@ module attributes {transform.with_named_sequence} {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
// expected-error @below {{failed to add pass or pass pipeline to pipeline: canonicalize}}
// expected-error @below {{<Pass-Options-Parser>: no such option invalid-option}}
- transform.apply_registered_pass "canonicalize" with options = "invalid-option=1" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = "invalid-option=1" to %1
+ : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -94,7 +96,9 @@ func.func @valid_pass_option() {
module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
- transform.apply_registered_pass "canonicalize" with options = "top-down=false" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = "top-down=false" to %1
+ : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -110,7 +114,9 @@ module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
//transform.apply_registered_pass "canonicalize" with options = "top-down=false,max-iterations=10" to %1 : (!transform.any_op) -> !transform.any_op
- transform.apply_registered_pass "canonicalize" with options = "top-down=false test-convergence=true" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = "top-down=false test-convergence=true" to %1
+ : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -125,7 +131,9 @@ func.func @valid_pass_options_as_list() {
module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
- transform.apply_registered_pass "canonicalize" with options = "top-down=false" "max-iterations=0" to %1 : (!transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = "top-down=false" "max-iterations=0" to %1
+ : (!transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -142,7 +150,9 @@ module attributes {transform.with_named_sequence} {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
%max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
%max_rewrites = transform.param.constant "max-num-rewrites=1" -> !transform.any_param
- %2 = transform.apply_registered_pass "canonicalize" with options = "top-down=false" %max_iter "test-convergence=true" %max_rewrites to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ %2 = transform.apply_registered_pass "canonicalize"
+ with options = "top-down=false" %max_iter "test-convergence=true" %max_rewrites to %1
+ : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -157,8 +167,10 @@ module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
%max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
- // expected-error @below {{expected at least one option (either a string or a param)}}
- %2 = transform.apply_registered_pass "canonicalize" with options = ["top-down=false" %max_iter] to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ // expected-error @+2 {{expected at least one option (either a string or a param)}}
+ %2 = transform.apply_registered_pass "canonicalize"
+ with options = ["top-down=false" %max_iter] to %1
+ : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -172,9 +184,10 @@ func.func @invalid_options_as_pairs() {
module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
- %max_iter = transform.param.constant "max-iterations=10" -> !transform.any_param
- // expected-error @below {{expected 'to'}}
- %2 = transform.apply_registered_pass "canonicalize" with options = "top-down=" false to %1 : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
+ // expected-error @+2 {{expected 'to'}}
+ %2 = transform.apply_registered_pass "canonicalize"
+ with options = "top-down=" false to %1
+ : (!transform.any_param, !transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -190,7 +203,9 @@ module attributes {transform.with_named_sequence} {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
%pass_options = transform.param.constant 42 -> !transform.any_param
// expected-error @below {{options passed as a param must be a string, got 42}}
- transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = %pass_options to %1
+ : (!transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
@@ -208,7 +223,9 @@ module attributes {transform.with_named_sequence} {
%y = transform.param.constant "y" -> !transform.any_param
%pass_options = transform.merge_handles %x, %y : !transform.any_param
// expected-error @below {{options passed as a param must have a single value associated, param 0 associates 2}}
- transform.apply_registered_pass "canonicalize" with options = %pass_options to %1 : (!transform.any_param, !transform.any_op) -> !transform.any_op
+ transform.apply_registered_pass "canonicalize"
+ with options = %pass_options to %1
+ : (!transform.any_param, !transform.any_op) -> !transform.any_op
transform.yield
}
}
>From 5e2344bd6469f1031778f4e4f913742c170a4847 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Fri, 6 Jun 2025 03:06:49 -0700
Subject: [PATCH 4/4] Fix
---
mlir/lib/Dialect/Transform/IR/TransformOps.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
index 32197322980f6..a0f9518e3d12f 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
@@ -924,7 +924,7 @@ static void printApplyRegisteredPassOptions(OpAsmPrinter &printer,
else if (auto strAttr = dyn_cast<StringAttr>(optionAttr))
printer.printAttribute(strAttr);
else
- assert(false && "each option should be either a StringAttr or UnitAttr");
+ llvm_unreachable("each option should be either a StringAttr or UnitAttr");
}
}
More information about the Mlir-commits
mailing list