manupa-arm commented on a change in pull request #8072: URL: https://github.com/apache/tvm/pull/8072#discussion_r655559775
########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { Review comment: Yes, I think there is precedence for both approaches. Lets not block this based on this :). Maybe its better to converge to a single policy when exposing member functions across the FFI. cc : @tqchen @jroesch . ########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { Review comment: Yes, I think there is precedence for both approaches. Lets not block this based on this :). Maybe its better to converge to a single policy when exposing member functions across the FFI. cc : @tqchen @jroesch . Though, if the intention is of this class to be used in python, I'd still prefer to have a documented interface in python -- it seems much easier to follow than the indirections via GetFunction. ########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { + public: + ModelLibraryFormatPrinter(bool show_meta_data, + const runtime::TypedPackedFunc<std::string(ObjectRef)>& annotate, + bool show_warning) + : text_printer_{show_meta_data, annotate, show_warning} {} + + const char* type_key() const override { return "model_library_format_printer"; } + + std::string Print(const ObjectRef& node) { + Doc doc; + doc << text_printer_.PrintFinal(node); + return doc.str(); + } + + PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) override { Review comment: If we are going with this approach, I feel we should limit this to just the lookup table of functions. i.e., its better to implement the lambda functions as separate functions Moreover, since this is main interface to runtime.Module, I think we should provide documentation of the functions and arguments. ########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { + public: + ModelLibraryFormatPrinter(bool show_meta_data, + const runtime::TypedPackedFunc<std::string(ObjectRef)>& annotate, + bool show_warning) + : text_printer_{show_meta_data, annotate, show_warning} {} + + const char* type_key() const override { return "model_library_format_printer"; } + + std::string Print(const ObjectRef& node) { + Doc doc; + doc << text_printer_.PrintFinal(node); + return doc.str(); + } + + PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) override { Review comment: If we are going with this approach, I feel we should limit this to just to the lookup ladder of functions. i.e., its better to implement the lambda functions as separate functions Moreover, since this is main interface to runtime.Module, I think we should provide documentation of the functions and arguments. ########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { + public: + ModelLibraryFormatPrinter(bool show_meta_data, + const runtime::TypedPackedFunc<std::string(ObjectRef)>& annotate, + bool show_warning) + : text_printer_{show_meta_data, annotate, show_warning} {} + + const char* type_key() const override { return "model_library_format_printer"; } + + std::string Print(const ObjectRef& node) { + Doc doc; + doc << text_printer_.PrintFinal(node); + return doc.str(); + } + + PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) override { + if (name == "print") { + return TypedPackedFunc<std::string(ObjectRef)>( + [sptr_to_self, this](ObjectRef node) { return Print(node); }); + } else if (name == "get_var_name") { + return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { + ICHECK_EQ(args.size(), 1) << "usage: get_var_name(Var v)"; + + std::string var_name; + if (text_printer_.GetVarName(args[0], &var_name)) { + *rv = var_name; + } + }); + } else { + return PackedFunc(); + } + } + + private: + TextPrinter text_printer_; +}; + +TVM_REGISTER_GLOBAL("tir.ModelLibraryFormatPrinter") Review comment: This seems fine as it works for all TIR -- I just got reminded that we moved MLF to micro for a different reason. ########## File path: src/printer/model_library_format_printer.cc ########## @@ -0,0 +1,75 @@ +/* + * 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 <tvm/runtime/module.h> +#include <tvm/runtime/registry.h> +#include <tvm/tir/var.h> + +#include "text_printer.h" + +namespace tvm { +namespace printer { + +class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode { + public: + ModelLibraryFormatPrinter(bool show_meta_data, + const runtime::TypedPackedFunc<std::string(ObjectRef)>& annotate, + bool show_warning) + : text_printer_{show_meta_data, annotate, show_warning} {} + + const char* type_key() const override { return "model_library_format_printer"; } + + std::string Print(const ObjectRef& node) { + Doc doc; + doc << text_printer_.PrintFinal(node); + return doc.str(); + } + + PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) override { Review comment: If we are going with this approach, I feel we should limit this to just to the lookup ladder of functions. i.e., its better to implement the lambda functions as separate functions Moreover, since this is main interface to runtime.Module, I think we should provide documentation of the functions and arguments -- maybe once the functions are seperated out, it could be the documentation of those functions. -- 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. For queries about this service, please contact Infrastructure at: [email protected]
