zhiics commented on a change in pull request #7002:
URL: https://github.com/apache/tvm/pull/7002#discussion_r534335252
##########
File path: src/relay/backend/build_module.cc
##########
@@ -493,18 +493,14 @@ class RelayBuildModule : public runtime::ModuleNode {
// If we cannot decide the target is LLVM, we create an empty
CSourceModule.
// The code content is initialized with ";" to prevent complaining
// from CSourceModuleNode::SaveToFile.
- ret_.mod = tvm::codegen::CSourceModuleCreate(";", "");
+ ret_.mod = tvm::codegen::CSourceModuleCreate(";", "", Array<String>{});
}
} else {
ret_.mod = tvm::build(lowered_funcs, target_host_);
}
- Array<tvm::runtime::Module> ext_mods =
graph_codegen_->GetExternalModules();
- // TODO(zhiics) We should be able to completely switch to MetadataModule no
- // matter whether there are external modules or not.
- if (!ext_mods.empty()) {
Review comment:
I think this would duplicate the param in the .so and .params file. I
had a plan to handle it but didn't have cycles.
##########
File path: src/relay/backend/contrib/codegen_c/codegen.cc
##########
@@ -215,20 +215,24 @@ class CodegenC : public
MemoizedExprTranslator<std::vector<Output>>, public Code
class CSourceCodegen : public CSourceModuleCodegenBase {
public:
- std::pair<std::string, Array<String>> GenCFunc(const Function& func) {
+ std::tuple<std::string, Array<String>, String, String> GenCFunc(const
Function& func) {
ICHECK(func.defined()) << "Input error: expect a Relay function.";
// Record the external symbol for runtime lookup.
auto sid = GetExtSymbol(func);
CodegenC builder(sid);
auto out = builder.VisitExpr(func->body);
- code_stream_ << builder.JIT(out);
-
- return {sid, builder.const_vars_};
Review comment:
I think sid is the same to ext_func_id_.
##########
File path: src/relay/backend/vm/compiler.cc
##########
@@ -1145,11 +1145,9 @@ void VMCompiler::Codegen() {
} else {
// There is no function handled by TVM. We create a virtual main module
// to make sure a DSO module will be also available.
- exec_->lib = codegen::CSourceModuleCreate(";", "");
- }
- if (!ext_mods.empty()) {
- exec_->lib = codegen::CreateMetadataModule(params_, exec_->lib, ext_mods);
Review comment:
same here
##########
File path: src/relay/backend/contrib/dnnl/codegen.cc
##########
@@ -413,7 +413,8 @@ class DNNLModuleCodegen : public CSourceModuleCodegenBase {
// Create a CSource module
const auto* pf = runtime::Registry::Get("runtime.CSourceModuleCreate");
ICHECK(pf != nullptr) << "Cannot find csource module to create the
external runtime module";
- return (*pf)(code, "c", sym, variables);
+ // TODO(@manupa-arm): pass the function names to enable system-lib creation
+ return (*pf)(code, "c", Array<String>{}, sym, variables);
Review comment:
Do we need an array? it seems that a csourcemodule is only one function
though.
##########
File path: src/relay/backend/contrib/codegen_c/codegen.cc
##########
@@ -259,18 +263,13 @@ class CSourceCodegen : public CSourceModuleCodegenBase {
)op_macro";
code_stream_ << operator_macro << "\n\n";
-
- ICHECK(ref->IsInstance<FunctionNode>());
- auto res = GenCFunc(Downcast<Function>(ref));
+ code_stream_ << std::get<3>(res);
std::string code = code_stream_.str();
- String sym = std::get<0>(res);
- Array<String> variables = std::get<1>(res);
-
// Create a CSource module
const auto* pf = runtime::Registry::Get("runtime.CSourceModuleCreate");
ICHECK(pf != nullptr) << "Cannot find csource module to create the
external runtime module";
- return (*pf)(code, "c", sym, variables);
+ return (*pf)(code, "c", Array<String>{func_name}, sym, variables);
Review comment:
I think we can use sym directly. It should be the same to func_name
----------------------------------------------------------------
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]
