[Mlir-commits] [mlir] fef3566 - [mlir] Pass Options ownership modifications (#110582)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Oct 1 09:48:54 PDT 2024
Author: Nikhil Kalra
Date: 2024-10-01T09:48:51-07:00
New Revision: fef3566a25ff0e34fb87339ba5e13eca17cec00f
URL: https://github.com/llvm/llvm-project/commit/fef3566a25ff0e34fb87339ba5e13eca17cec00f
DIFF: https://github.com/llvm/llvm-project/commit/fef3566a25ff0e34fb87339ba5e13eca17cec00f.diff
LOG: [mlir] Pass Options ownership modifications (#110582)
This change makes two (related) changes:
First, it updates the tablegen option for `ListOption` to emit a
`SmallVector` instead of an `ArrayRef`. This brings `ListOption` more
inline with the traditional `Option`, where values are typically
provided using types that have storage. After this change, all options
should be fully owned by a Pass' `Options` object after it has been
fully constructed, unless the underlying type of the `Option` explicitly
indicates otherwise.
Second, it updates the generated constructors for Passes to consume
options by value instead of reference, and prefers moving options into
the pass itself. This should be more efficient for non-trivial options
objects, where the previous interface forced a copy to be materialized.
Now, at worst case the API materializes a copy (no worse than before);
at best-case, all options objects are moved into place. Ideally, we
could update the Pass constructor to take an r-value reference to the
Options object instead, but this approach will require numerous changes
to existing passes and their factory functions.
---------
Authored-by: Nikhil Kalra <nkalra at apple.com>
Added:
Modified:
mlir/include/mlir/Transforms/Passes.h
mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp
mlir/tools/mlir-tblgen/PassGen.cpp
mlir/unittests/TableGen/PassGenTest.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Transforms/Passes.h b/mlir/include/mlir/Transforms/Passes.h
index 8e4a43c3f24586..5c977055e95dc8 100644
--- a/mlir/include/mlir/Transforms/Passes.h
+++ b/mlir/include/mlir/Transforms/Passes.h
@@ -15,6 +15,7 @@
#define MLIR_TRANSFORMS_PASSES_H
#include "mlir/Pass/Pass.h"
+#include "mlir/Pass/PassManager.h"
#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
#include "mlir/Transforms/LocationSnapshot.h"
#include "mlir/Transforms/ViewOpGraph.h"
diff --git a/mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp b/mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp
index 70fd9bc0a1e68f..b61218bb7f1af6 100644
--- a/mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp
+++ b/mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp
@@ -44,7 +44,8 @@ struct NarrowingPattern : OpRewritePattern<SourceOp> {
NarrowingPattern(MLIRContext *ctx, const ArithIntNarrowingOptions &options,
PatternBenefit benefit = 1)
: OpRewritePattern<SourceOp>(ctx, benefit),
- supportedBitwidths(options.bitwidthsSupported) {
+ supportedBitwidths(options.bitwidthsSupported.begin(),
+ options.bitwidthsSupported.end()) {
assert(!supportedBitwidths.empty() && "Invalid options");
assert(!llvm::is_contained(supportedBitwidths, 0) && "Invalid bitwidth");
llvm::sort(supportedBitwidths);
@@ -757,7 +758,8 @@ struct ArithIntNarrowingPass final
MLIRContext *ctx = op->getContext();
RewritePatternSet patterns(ctx);
populateArithIntNarrowingPatterns(
- patterns, ArithIntNarrowingOptions{bitwidthsSupported});
+ patterns, ArithIntNarrowingOptions{
+ llvm::to_vector_of<unsigned>(bitwidthsSupported)});
if (failed(applyPatternsAndFoldGreedily(op, std::move(patterns))))
signalPassFailure();
}
diff --git a/mlir/tools/mlir-tblgen/PassGen.cpp b/mlir/tools/mlir-tblgen/PassGen.cpp
index 90aa67115a4007..655843f26201ac 100644
--- a/mlir/tools/mlir-tblgen/PassGen.cpp
+++ b/mlir/tools/mlir-tblgen/PassGen.cpp
@@ -97,7 +97,7 @@ static void emitPassOptionsStruct(const Pass &pass, raw_ostream &os) {
std::string type = opt.getType().str();
if (opt.isListOption())
- type = "::llvm::ArrayRef<" + type + ">";
+ type = "::llvm::SmallVector<" + type + ">";
os.indent(2) << llvm::formatv("{0} {1}", type, opt.getCppVariableName());
@@ -128,8 +128,8 @@ static void emitPassDecls(const Pass &pass, raw_ostream &os) {
// Declaration of the constructor with options.
if (ArrayRef<PassOption> options = pass.getOptions(); !options.empty())
- os << llvm::formatv("std::unique_ptr<::mlir::Pass> create{0}(const "
- "{0}Options &options);\n",
+ os << llvm::formatv("std::unique_ptr<::mlir::Pass> create{0}("
+ "{0}Options options);\n",
passName);
}
@@ -236,7 +236,7 @@ namespace impl {{
const char *const friendDefaultConstructorWithOptionsDeclTemplate = R"(
namespace impl {{
- std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options);
+ std::unique_ptr<::mlir::Pass> create{0}({0}Options options);
} // namespace impl
)";
@@ -247,8 +247,8 @@ const char *const friendDefaultConstructorDefTemplate = R"(
)";
const char *const friendDefaultConstructorWithOptionsDefTemplate = R"(
- friend std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options) {{
- return std::make_unique<DerivedT>(options);
+ friend std::unique_ptr<::mlir::Pass> create{0}({0}Options options) {{
+ return std::make_unique<DerivedT>(std::move(options));
}
)";
@@ -259,8 +259,8 @@ std::unique_ptr<::mlir::Pass> create{0}() {{
)";
const char *const defaultConstructorWithOptionsDefTemplate = R"(
-std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options) {{
- return impl::create{0}(options);
+std::unique_ptr<::mlir::Pass> create{0}({0}Options options) {{
+ return impl::create{0}(std::move(options));
}
)";
@@ -326,10 +326,10 @@ static void emitPassDefs(const Pass &pass, raw_ostream &os) {
if (ArrayRef<PassOption> options = pass.getOptions(); !options.empty()) {
os.indent(2) << llvm::formatv(
- "{0}Base(const {0}Options &options) : {0}Base() {{\n", passName);
+ "{0}Base({0}Options options) : {0}Base() {{\n", passName);
for (const PassOption &opt : pass.getOptions())
- os.indent(4) << llvm::formatv("{0} = options.{0};\n",
+ os.indent(4) << llvm::formatv("{0} = std::move(options.{0});\n",
opt.getCppVariableName());
os.indent(2) << "}\n";
diff --git a/mlir/unittests/TableGen/PassGenTest.cpp b/mlir/unittests/TableGen/PassGenTest.cpp
index 859eba7e517869..27f2fa06542961 100644
--- a/mlir/unittests/TableGen/PassGenTest.cpp
+++ b/mlir/unittests/TableGen/PassGenTest.cpp
@@ -72,8 +72,7 @@ TEST(PassGenTest, PassOptions) {
TestPassWithOptionsOptions options;
options.testOption = 57;
- llvm::SmallVector<int64_t, 2> testListOption = {1, 2};
- options.testListOption = testListOption;
+ options.testListOption = {1, 2};
const auto unwrap = [](const std::unique_ptr<mlir::Pass> &pass) {
return static_cast<const TestPassWithOptions *>(pass.get());
More information about the Mlir-commits
mailing list