tqchen commented on code in PR #230:
URL: https://github.com/apache/tvm-ffi/pull/230#discussion_r2527389809
##########
include/tvm/ffi/function.h:
##########
@@ -840,13 +840,56 @@ inline int32_t TypeKeyToIndex(std::string_view type_key) {
return type_index;
}
+/// \cond Doxygen_Suppress
+// Internal implementation macros used by TVM_FFI_DLL_EXPORT_TYPED_FUNC and
related macros.
+// These should not be used directly; use the public macros instead.
+
+// Internal implementation macro that generates the C ABI wrapper function
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC_IMPL_(ExportName, Function)
\
+ extern "C" {
\
+ TVM_FFI_DLL_EXPORT int __tvm_ffi_##ExportName(void* self, const TVMFFIAny*
args, \
+ int32_t num_args, TVMFFIAny*
result) { \
+ TVM_FFI_SAFE_CALL_BEGIN();
\
+ using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;
\
+ static std::string name = #ExportName;
\
+ ::tvm::ffi::details::unpack_call<typename FuncInfo::RetType>(
\
+ std::make_index_sequence<FuncInfo::num_args>{}, &name, Function,
\
+ reinterpret_cast<const ::tvm::ffi::AnyView*>(args), num_args,
\
+ reinterpret_cast<::tvm::ffi::Any*>(result));
\
+ TVM_FFI_SAFE_CALL_END();
\
+ }
\
+ }
+
+// Internal implementation macro that optionally generates metadata export
function
+#ifdef TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA
Review Comment:
TVM_FFI_DLL_EXPORT_INCLUDE_METADATA, predefine the macro as a flag value 0/1
##########
include/tvm/ffi/function.h:
##########
@@ -840,13 +840,56 @@ inline int32_t TypeKeyToIndex(std::string_view type_key) {
return type_index;
}
+/// \cond Doxygen_Suppress
+// Internal implementation macros used by TVM_FFI_DLL_EXPORT_TYPED_FUNC and
related macros.
+// These should not be used directly; use the public macros instead.
+
+// Internal implementation macro that generates the C ABI wrapper function
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC_IMPL_(ExportName, Function)
\
+ extern "C" {
\
+ TVM_FFI_DLL_EXPORT int __tvm_ffi_##ExportName(void* self, const TVMFFIAny*
args, \
+ int32_t num_args, TVMFFIAny*
result) { \
+ TVM_FFI_SAFE_CALL_BEGIN();
\
+ using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;
\
+ static std::string name = #ExportName;
\
+ ::tvm::ffi::details::unpack_call<typename FuncInfo::RetType>(
\
+ std::make_index_sequence<FuncInfo::num_args>{}, &name, Function,
\
+ reinterpret_cast<const ::tvm::ffi::AnyView*>(args), num_args,
\
+ reinterpret_cast<::tvm::ffi::Any*>(result));
\
+ TVM_FFI_SAFE_CALL_END();
\
+ }
\
+ }
+
+// Internal implementation macro that optionally generates metadata export
function
+#ifdef TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA_IMPL_(ExportName, Function)
\
+ inline ::tvm::ffi::String __tvm_ffi_get_metadata_##ExportName() {
\
+ using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;
\
+ /* Generate metadata on demand */
\
+ std::ostringstream os;
\
+ os << R"({"type_schema":)"
\
Review Comment:
need to include sstream in function.h because we need to have this
##########
include/tvm/ffi/function_details.h:
##########
@@ -253,6 +253,41 @@ struct TypeSchemaImpl {
}
};
+/*!
+ * \brief Helper to detect const-ness of a type parameter.
+ * Used for memory effect annotation in function metadata.
+ */
+template <typename T>
+struct IsConstParam {
Review Comment:
this is the part worth discussing, since normally the arg const in c++ is
not a good indicative factor of arg_const in desirable metadata (most libs
won't obey this).
I would be conservative first and not include it, then think about asking
users to explicit annotate. What we really want to say is an op take some as
input and muates some others. That metadata perhaps should come from outside,
or being explicitly annotated
Better to be conservative than having the wrong one. Let us first remove
this part
cc @junrushao @Kathryn-cat
##########
python/tvm_ffi/module.py:
##########
@@ -170,6 +180,93 @@ def get_function(self, name: str, query_imports: bool =
False) -> core.Function:
raise AttributeError(f"Module has no function '{name}'")
return func
+ def get_function_metadata(
+ self, name: str, query_imports: bool = False
+ ) -> dict[str, Any] | None:
+ """Get metadata for a function exported from the module.
+
+ This retrieves metadata for functions exported via
TVM_FFI_DLL_EXPORT_TYPED_FUNC
+ and when TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA is on, which includes
type schema
+ and const-ness information.
+
+ Parameters
+ ----------
+ name
+ The name of the function
+
+ query_imports
+ Whether to also query modules imported by this module.
+
+ Returns
+ -------
+ metadata
+ A dictionary containing function metadata. The ``type_schema``
field
+ encodes the callable signature.
+
+ Examples
+ --------
+ .. code-block:: python
+
+ import tvm_ffi
+ from tvm_ffi.core import TypeSchema
+ import json
+
+ mod = tvm_ffi.load_module("add_one_cpu.so")
+ metadata = mod.get_function_metadata("add_one_cpu")
+ schema = TypeSchema.from_json_str(metadata["type_schema"])
+ print(schema) # Shows function signature
+
+ See Also
+ --------
+ :py:func:`tvm_ffi.get_global_func_metadata`
+ Get metadata for global registry functions.
+
+ """
+ metadata_str = _ffi_api.ModuleGetFunctionMetadata(self, name,
query_imports)
+ if metadata_str is None:
+ return None
+ return json.loads(metadata_str)
+
+ def get_function_doc(self, name: str, query_imports: bool = False) -> str
| None:
Review Comment:
make sure we can eager auto attach the doc to function, so likely python
user do not need this function
##########
src/ffi/testing/testing.cc:
##########
@@ -282,6 +284,86 @@ TVM_FFI_STATIC_INIT_BLOCK() {
namespace tvm {
namespace ffi {
+//
-----------------------------------------------------------------------------
+// Implementation functions for schema testing
+// These are registered via both GlobalDef and TVM_FFI_DLL_EXPORT_TYPED_FUNC
+//
-----------------------------------------------------------------------------
+namespace schema_test_impl {
+
Review Comment:
move these tests files into a cpp.load_inline test instead, so we don't have
to blow up the testing. Given we use the same mechanism as the global one, we
don't need to test so many functions
##########
include/tvm/ffi/function.h:
##########
@@ -863,22 +906,66 @@ inline int32_t TypeKeyToIndex(std::string_view type_key) {
* });
* \endcode
*
- * \note The final symbol name is `__tvm_ffi_<ExportName>`.
+ * \note The final symbol names are:
+ * - `__tvm_ffi_<ExportName>` (function)
+ * - `__tvm_ffi__metadata_<ExportName>` (metadata - only when
+ * TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA is defined)
*/
-#define TVM_FFI_DLL_EXPORT_TYPED_FUNC(ExportName, Function)
\
- extern "C" {
\
- TVM_FFI_DLL_EXPORT int __tvm_ffi_##ExportName(void* self, const TVMFFIAny*
args, \
- int32_t num_args, TVMFFIAny*
result) { \
- TVM_FFI_SAFE_CALL_BEGIN();
\
- using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;
\
- static std::string name = #ExportName;
\
- ::tvm::ffi::details::unpack_call<typename FuncInfo::RetType>(
\
- std::make_index_sequence<FuncInfo::num_args>{}, &name, Function,
\
- reinterpret_cast<const ::tvm::ffi::AnyView*>(args), num_args,
\
- reinterpret_cast<::tvm::ffi::Any*>(result));
\
- TVM_FFI_SAFE_CALL_END();
\
- }
\
- }
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC(ExportName, Function) \
+ TVM_FFI_DLL_EXPORT_TYPED_FUNC_IMPL_(ExportName, Function) \
+ TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA_IMPL_(ExportName, Function)
+
+/*!
+ * \brief Export typed function with documentation as SafeCallType symbols.
+ *
+ * This is an extended version of TVM_FFI_DLL_EXPORT_TYPED_FUNC that also
exports
+ * a documentation string. The docstring can be used by stub generators and
+ * documentation tools.
+ *
+ * \param ExportName The symbol name to be exported.
+ * \param Function The typed function.
+ * \param DocString The documentation string (C string literal).
+ *
+ * \sa ffi::TypedFunction, TVM_FFI_DLL_EXPORT_TYPED_FUNC
+ *
+ * \code
+ *
+ * int Add(int a, int b) {
+ * return a + b;
+ * }
+ *
+ * TVM_FFI_DLL_EXPORT_TYPED_FUNC_WITH_DOC(
+ * add,
+ * Add,
+ * "Add two integers and return the sum.\n"
+ * "\n"
+ * "Parameters\n"
+ * "----------\n"
+ * "a : int\n"
+ * " First integer\n"
+ * "b : int\n"
+ * " Second integer\n"
+ * "\n"
+ * "Returns\n"
+ * "-------\n"
+ * "result : int\n"
+ * " Sum of a and b");
+ *
+ * \endcode
+ *
+ * \note The final symbol names are:
+ * - `__tvm_ffi_<ExportName>` (function)
+ * - `__tvm_ffi__metadata_<ExportName>` (metadata - only when
+ * TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA is defined)
+ * - `__tvm_ffi__doc_<ExportName>` (docstring)
+ */
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC_WITH_DOC(ExportName, Function,
DocString) \
Review Comment:
how about we just do TVM_FFI_DLL_EXPORT_TYPED_FUNC_DOC, ExportName, DocString
##########
python/tvm_ffi/module.py:
##########
@@ -170,6 +180,93 @@ def get_function(self, name: str, query_imports: bool =
False) -> core.Function:
raise AttributeError(f"Module has no function '{name}'")
return func
+ def get_function_metadata(
Review Comment:
we can keep this function since metadata is not as common, make sure it
aligns with function in global one
##########
include/tvm/ffi/function.h:
##########
@@ -840,13 +840,56 @@ inline int32_t TypeKeyToIndex(std::string_view type_key) {
return type_index;
}
+/// \cond Doxygen_Suppress
+// Internal implementation macros used by TVM_FFI_DLL_EXPORT_TYPED_FUNC and
related macros.
+// These should not be used directly; use the public macros instead.
+
+// Internal implementation macro that generates the C ABI wrapper function
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC_IMPL_(ExportName, Function)
\
+ extern "C" {
\
+ TVM_FFI_DLL_EXPORT int __tvm_ffi_##ExportName(void* self, const TVMFFIAny*
args, \
+ int32_t num_args, TVMFFIAny*
result) { \
+ TVM_FFI_SAFE_CALL_BEGIN();
\
+ using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;
\
+ static std::string name = #ExportName;
\
+ ::tvm::ffi::details::unpack_call<typename FuncInfo::RetType>(
\
+ std::make_index_sequence<FuncInfo::num_args>{}, &name, Function,
\
+ reinterpret_cast<const ::tvm::ffi::AnyView*>(args), num_args,
\
+ reinterpret_cast<::tvm::ffi::Any*>(result));
\
+ TVM_FFI_SAFE_CALL_END();
\
+ }
\
+ }
+
+// Internal implementation macro that optionally generates metadata export
function
+#ifdef TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA
Review Comment:
then we use `#if TVM_FFI_DLL_EXPORT_INCLUDE_METADATA`
##########
tests/cpp/extra/test_module.cc:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include <gtest/gtest.h>
+#include <tvm/ffi/container/array.h>
+#include <tvm/ffi/container/map.h>
+#include <tvm/ffi/extra/json.h>
+#include <tvm/ffi/extra/module.h>
+#include <tvm/ffi/function.h>
+
+namespace {
+
+using namespace tvm::ffi;
+
+String GetTestingLibFilename() {
Review Comment:
for this case, let us simply run the test from the python side, to keep
things simple and remove this cxx test
##########
tests/cpp/test_function.cc:
##########
@@ -285,6 +289,21 @@ int invoke_testing_add1(int x) {
TEST(Func, InvokeExternC) { EXPECT_EQ(invoke_testing_add1(1), 2); }
+extern "C" int __tvm_ffi__metadata_testing_add1(void*, const TVMFFIAny*,
int32_t, TVMFFIAny*);
+
+TEST(Func, StaticLinkingMetadata) {
+ String metadata_str =
+ Function::InvokeExternC(nullptr,
__tvm_ffi__metadata_testing_add1).cast<String>();
+ Map<String, Any> metadata = json::Parse(metadata_str).cast<Map<String,
Any>>();
+ EXPECT_TRUE(metadata.count("type_schema"));
Review Comment:
we can keep this one test here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
