-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][Standalone] use narrow registration instead of RegisterEverything #160469
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
Conversation
eb6cf81
to
cf4b971
Compare
859ae97
to
370b09d
Compare
@llvm/pr-subscribers-mlir Author: Maksim Levental (makslevental) ChangesThis PR cleans up a long-standing TODO by exposing Full diff: https://github.com/llvm/llvm-project/pull/160469.diff 10 Files Affected:
diff --git a/mlir/examples/standalone/python/CMakeLists.txt b/mlir/examples/standalone/python/CMakeLists.txt
index 5404128e5fa27..8ac8f501470ba 100644
--- a/mlir/examples/standalone/python/CMakeLists.txt
+++ b/mlir/examples/standalone/python/CMakeLists.txt
@@ -30,6 +30,10 @@ declare_mlir_python_extension(StandalonePythonSources.Pybind11Extension
PRIVATE_LINK_LIBS
LLVMSupport
EMBED_CAPI_LINK_LIBS
+ MLIRCAPIIR
+ MLIRCAPIArith
+ MLIRCAPIBuiltin
+ MLIRCAPITransforms
StandaloneCAPI
PYTHON_BINDINGS_LIBRARY pybind11
)
@@ -42,6 +46,10 @@ declare_mlir_python_extension(StandalonePythonSources.NanobindExtension
PRIVATE_LINK_LIBS
LLVMSupport
EMBED_CAPI_LINK_LIBS
+ MLIRCAPIIR
+ MLIRCAPIArith
+ MLIRCAPIBuiltin
+ MLIRCAPITransforms
StandaloneCAPI
PYTHON_BINDINGS_LIBRARY nanobind
)
@@ -58,9 +66,6 @@ add_mlir_python_common_capi_library(StandalonePythonCAPI
RELATIVE_INSTALL_ROOT "../../../.."
DECLARED_SOURCES
StandalonePythonSources
- # TODO: Remove this in favor of showing fine grained registration once
- # available.
- MLIRPythonExtension.RegisterEverything
MLIRPythonSources.Core
MLIRPythonSources.Dialects.builtin
)
@@ -130,9 +135,6 @@ declare_mlir_python_sources(
)
set(_declared_sources
StandalonePythonSources
- # TODO: Remove this in favor of showing fine grained registration once
- # available.
- MLIRPythonExtension.RegisterEverything
MLIRPythonSources.Core
MLIRPythonSources.Dialects.builtin
)
diff --git a/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp b/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp
index e06ec3b6472b8..d93bfcc91065d 100644
--- a/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp
+++ b/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp
@@ -10,6 +10,8 @@
//===----------------------------------------------------------------------===//
#include "Standalone-c/Dialects.h"
+#include "mlir-c/Dialect/Arith.h"
+#include "mlir-c/Dialect/Builtin.h"
#include "mlir/Bindings/Python/Nanobind.h"
#include "mlir/Bindings/Python/NanobindAdaptors.h"
@@ -22,17 +24,24 @@ NB_MODULE(_standaloneDialectsNanobind, m) {
auto standaloneM = m.def_submodule("standalone");
standaloneM.def(
- "register_dialect",
+ "register_dialects",
[](MlirContext context, bool load) {
- MlirDialectHandle handle = mlirGetDialectHandle__standalone__();
- mlirDialectHandleRegisterDialect(handle, context);
+ MlirDialectHandle arithHandle = mlirGetDialectHandle__arith__();
+ MlirDialectHandle builtinHandle = mlirGetDialectHandle__builtin__();
+ MlirDialectHandle standaloneHandle =
+ mlirGetDialectHandle__standalone__();
+ mlirDialectHandleRegisterDialect(arithHandle, context);
+ mlirDialectHandleRegisterDialect(builtinHandle, context);
+ mlirDialectHandleRegisterDialect(standaloneHandle, context);
if (load) {
- mlirDialectHandleLoadDialect(handle, context);
+ mlirDialectHandleLoadDialect(arithHandle, context);
+ mlirDialectHandleRegisterDialect(builtinHandle, context);
+ mlirDialectHandleRegisterDialect(standaloneHandle, context);
}
},
nb::arg("context").none() = nb::none(), nb::arg("load") = true,
// clang-format off
- nb::sig("def register_dialect(context: " MAKE_MLIR_PYTHON_QUALNAME("ir.Context") ", load: bool = True) -> None")
+ nb::sig("def register_dialects(context: " MAKE_MLIR_PYTHON_QUALNAME("ir.Context") ", load: bool = True) -> None")
// clang-format on
);
}
diff --git a/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp b/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
index 397db4c20e743..530ce316049b6 100644
--- a/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
+++ b/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
@@ -10,6 +10,8 @@
//===----------------------------------------------------------------------===//
#include "Standalone-c/Dialects.h"
+#include "mlir-c/Dialect/Arith.h"
+#include "mlir-c/Dialect/Builtin.h"
#include "mlir/Bindings/Python/PybindAdaptors.h"
using namespace mlir::python::adaptors;
@@ -21,12 +23,19 @@ PYBIND11_MODULE(_standaloneDialectsPybind11, m) {
auto standaloneM = m.def_submodule("standalone");
standaloneM.def(
- "register_dialect",
+ "register_dialects",
[](MlirContext context, bool load) {
- MlirDialectHandle handle = mlirGetDialectHandle__standalone__();
- mlirDialectHandleRegisterDialect(handle, context);
+ MlirDialectHandle arithHandle = mlirGetDialectHandle__arith__();
+ MlirDialectHandle builtinHandle = mlirGetDialectHandle__builtin__();
+ MlirDialectHandle standaloneHandle =
+ mlirGetDialectHandle__standalone__();
+ mlirDialectHandleRegisterDialect(arithHandle, context);
+ mlirDialectHandleRegisterDialect(builtinHandle, context);
+ mlirDialectHandleRegisterDialect(standaloneHandle, context);
if (load) {
- mlirDialectHandleLoadDialect(handle, context);
+ mlirDialectHandleLoadDialect(arithHandle, context);
+ mlirDialectHandleRegisterDialect(builtinHandle, context);
+ mlirDialectHandleRegisterDialect(standaloneHandle, context);
}
},
py::arg("context") = py::none(), py::arg("load") = true);
diff --git a/mlir/examples/standalone/test/CAPI/CMakeLists.txt b/mlir/examples/standalone/test/CAPI/CMakeLists.txt
index eaa6cfc102c73..7f4b780ac6efe 100644
--- a/mlir/examples/standalone/test/CAPI/CMakeLists.txt
+++ b/mlir/examples/standalone/test/CAPI/CMakeLists.txt
@@ -6,9 +6,8 @@ add_mlir_aggregate(StandaloneCAPITestLib
SHARED
EMBED_LIBS
MLIRCAPIIR
- # TODO: Remove this in favor of showing fine grained dialect registration
- # (once available).
- MLIRCAPIRegisterEverything
+ MLIRCAPIArith
+ MLIRCAPIBuiltin
StandaloneCAPI
)
diff --git a/mlir/examples/standalone/test/CAPI/standalone-capi-test.c b/mlir/examples/standalone/test/CAPI/standalone-capi-test.c
index 54f3ca7f7ff14..20e86d39cdc82 100644
--- a/mlir/examples/standalone/test/CAPI/standalone-capi-test.c
+++ b/mlir/examples/standalone/test/CAPI/standalone-capi-test.c
@@ -12,21 +12,14 @@
#include <stdio.h>
#include "Standalone-c/Dialects.h"
+#include "mlir-c/Dialect/Arith.h"
+#include "mlir-c/Dialect/Builtin.h"
#include "mlir-c/IR.h"
-#include "mlir-c/RegisterEverything.h"
-
-static void registerAllUpstreamDialects(MlirContext ctx) {
- MlirDialectRegistry registry = mlirDialectRegistryCreate();
- mlirRegisterAllDialects(registry);
- mlirContextAppendDialectRegistry(ctx, registry);
- mlirDialectRegistryDestroy(registry);
-}
int main(int argc, char **argv) {
MlirContext ctx = mlirContextCreate();
- // TODO: Create the dialect handles for the builtin dialects and avoid this.
- // This adds dozens of MB of binary size over just the standalone dialect.
- registerAllUpstreamDialects(ctx);
+ mlirDialectHandleRegisterDialect(mlirGetDialectHandle__arith__(), ctx);
+ mlirDialectHandleRegisterDialect(mlirGetDialectHandle__builtin__(), ctx);
mlirDialectHandleRegisterDialect(mlirGetDialectHandle__standalone__(), ctx);
MlirModule module = mlirModuleCreateParse(
diff --git a/mlir/examples/standalone/test/python/smoketest.py b/mlir/examples/standalone/test/python/smoketest.py
index bd40c65d16164..26d84fd63e947 100644
--- a/mlir/examples/standalone/test/python/smoketest.py
+++ b/mlir/examples/standalone/test/python/smoketest.py
@@ -3,7 +3,6 @@
import sys
from mlir_standalone.ir import *
-from mlir_standalone.dialects import builtin as builtin_d
if sys.argv[1] == "pybind11":
from mlir_standalone.dialects import standalone_pybind11 as standalone_d
@@ -14,7 +13,7 @@
with Context():
- standalone_d.register_dialect()
+ standalone_d.register_dialects()
module = Module.parse(
"""
%0 = arith.constant 2 : i32
diff --git a/mlir/include/mlir-c/Dialect/Builtin.h b/mlir/include/mlir-c/Dialect/Builtin.h
new file mode 100644
index 0000000000000..c5d958249b36f
--- /dev/null
+++ b/mlir/include/mlir-c/Dialect/Builtin.h
@@ -0,0 +1,33 @@
+//===-- mlir-c/Dialect/Builtin.h - C API for Builtin dialect ------*- C -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
+// Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This header declares the C interface for registering and accessing the
+// Builtin dialect. A dialect should be registered with a context to make it
+// available to users of the context. These users must load the dialect
+// before using any of its attributes, operations or types. Parser and pass
+// manager can load registered dialects automatically.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_C_DIALECT_BUILTIN_H
+#define MLIR_C_DIALECT_BUILTIN_H
+
+#include "mlir-c/IR.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+MLIR_DECLARE_CAPI_DIALECT_REGISTRATION(Builtin, builtin);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // MLIR_C_DIALECT_BUILTIN_H
diff --git a/mlir/lib/CAPI/Dialect/Builtin.cpp b/mlir/lib/CAPI/Dialect/Builtin.cpp
new file mode 100644
index 0000000000000..dd44c8db84a14
--- /dev/null
+++ b/mlir/lib/CAPI/Dialect/Builtin.cpp
@@ -0,0 +1,14 @@
+//===- Builtin.cpp - C Interface for Builtin dialect
+//--------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir-c/Dialect/Builtin.h"
+#include "mlir/CAPI/Registration.h"
+#include "mlir/IR/BuiltinDialect.h"
+
+MLIR_DEFINE_CAPI_DIALECT_REGISTRATION(Builtin, builtin, mlir::BuiltinDialect)
diff --git a/mlir/lib/CAPI/Dialect/CMakeLists.txt b/mlir/lib/CAPI/Dialect/CMakeLists.txt
index bb1fdf8be3c8f..0b8a542b8826d 100644
--- a/mlir/lib/CAPI/Dialect/CMakeLists.txt
+++ b/mlir/lib/CAPI/Dialect/CMakeLists.txt
@@ -31,6 +31,14 @@ add_mlir_upstream_c_api_library(MLIRCAPIAsync
MLIRPass
)
+add_mlir_upstream_c_api_library(MLIRCAPIBuiltin
+ Builtin.cpp
+
+ PARTIAL_SOURCES_INTENDED
+ LINK_LIBS PUBLIC
+ MLIRCAPIIR
+)
+
add_mlir_upstream_c_api_library(MLIRCAPIControlFlow
ControlFlow.cpp
diff --git a/mlir/test/CMakeLists.txt b/mlir/test/CMakeLists.txt
index 628adcfb6e285..2b846a9d3dc9f 100644
--- a/mlir/test/CMakeLists.txt
+++ b/mlir/test/CMakeLists.txt
@@ -123,7 +123,14 @@ set(MLIR_TEST_DEPENDS
tblgen-to-irdl
)
if(NOT MLIR_STANDALONE_BUILD)
- list(APPEND MLIR_TEST_DEPENDS FileCheck count not split-file yaml2obj)
+ list(APPEND
+ MLIR_TEST_DEPENDS
+ FileCheck count not
+ split-file yaml2obj
+ MLIRCAPIIR
+ MLIRCAPIArith
+ MLIRCAPIBuiltin
+ )
endif()
set(MLIR_TEST_DEPENDS ${MLIR_TEST_DEPENDS}
|
370b09d
to
5187ae7
Compare
mlir/test/CMakeLists.txt
Outdated
endif() | ||
# Examples/standalone/test.toy (vis-a-vis the standalone example) depends on these. | ||
if(LLVM_INCLUDE_EXAMPLES) | ||
list(APPEND MLIR_TEST_DEPENDS MLIRCAPIIR MLIRCAPIArith) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question: these aren't already dependencies of the various mlir-capi-*-test
listed above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea they should be right? I can try it again but I added them of course because running LIT_FILTER=test.toy ninja check-mlir
would fail with errors about missing the corresponding object files. Possibly something about CMake task graph? But I'll try again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah it's just MLIRCAPIArith
which is needed here (since there are no acutal Arith C API tests that depend on that lib): https://github.com/llvm/llvm-project/actions/runs/17987615036/job/51169652084#step:3:7493
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the one question I had left.
0429f84
to
b2827e3
Compare
…ing (llvm#160469) This PR cleans up a long-standing TODO by avoiding `MLIRPythonExtension.RegisterEverything` in the Standalone example and registering the necessary dialects explicitly instead.
This PR cleans up a long-standing TODO by avoiding
MLIRPythonExtension.RegisterEverything
in the Standalone example and registering the necessary dialects explicitly instead.