manupa-arm commented on a change in pull request #7002:
URL: https://github.com/apache/tvm/pull/7002#discussion_r546196370



##########
File path: src/target/source/source_module.cc
##########
@@ -46,40 +48,68 @@ using runtime::SaveBinaryToFile;
  *        codegens, such as graph runtime codegen and the vm compiler.
  *
  * \param params The metadata for initialization of all modules.
- * \param dso_module The DSO module that contains TVM primitives.
- * \param modules The submodules that will be wrapped, e.g. CSource modules 
that
- *        contain vendor library calls or customized runtime modules.
- *
+ * \param target_module the internal module that is compiled by tvm.
+ * \param ext_modules The external modules that needs to be imported inside 
the metadata
+ * module(s).
+ * \param target The target that all the modules are compiled for
  * \return The created metadata module that manages initialization of metadata.
  */
 runtime::Module CreateMetadataModule(
     const std::unordered_map<std::string, runtime::NDArray>& params,
-    const runtime::Module& dso_module, const Array<runtime::Module>& modules) {
+    tvm::runtime::Module target_module, const Array<runtime::Module>& 
ext_modules, Target target) {
+  Array<tvm::runtime::Module> csource_modules;
+  Array<tvm::runtime::Module> binary_modules;
+
+  auto DSOExportable = [](tvm::runtime::Module& mod) {
+    return !std::strcmp(mod->type_key(), "llvm") || 
!std::strcmp(mod->type_key(), "c");
+  };
+
   // Wrap all submodules in the initialization wrapper.
   std::unordered_map<std::string, std::vector<std::string>> sym_metadata;
-  for (runtime::Module it : modules) {
-    auto pf_sym = it.GetFunction("get_symbol");
-    auto pf_var = it.GetFunction("get_const_vars");
+  for (tvm::runtime::Module mod : ext_modules) {
+    auto pf_sym = mod.GetFunction("get_symbol");
+    auto pf_var = mod.GetFunction("get_const_vars");
+    std::vector<std::string> arrays;
     if (pf_sym != nullptr && pf_var != nullptr) {
       String symbol = pf_sym();
       Array<String> variables = pf_var();
-      std::vector<std::string> arrays;
       for (size_t i = 0; i < variables.size(); i++) {
         arrays.push_back(variables[i].operator std::string());
       }
       ICHECK_EQ(sym_metadata.count(symbol), 0U) << "Found duplicated symbol: " 
<< symbol;
       sym_metadata[symbol] = arrays;
     }
+    // We only need loading of serialized constant data
+    // if there are constants present and required by the
+    // runtime module to be initialized by the binary
+    // metadata module. If not rest of the modules are
+    // wrapped in c-source metadata module.
+
+    // TODO(@manupa-arm) : we should be able to use csource_metadata
+    // if the variables are empty when all the runtime modules implement 
get_func_names
+    if (arrays.empty() && DSOExportable(mod) && target->kind->name == "c") {
+      csource_modules.push_back(mod);

Review comment:
       Yes, that is correct.
   
   We would only need the current MetadataModule (which is non-DSOExportable) 
if the runtime module's requires the constants to be copied in the init process 
-- which is something we dont want in bare-metal. In-fact if the non of 
external modules have constants extracted (e.g., ethos-n and ethos-u will be 
managing their own constants -- truly with the current approach each sub module 
are copied with their constants from the MetadataModule by the MetadataModule), 
we dont even want MetadataModule (which is non-DSOExportable) get imported to 
the module hierarchy.
   
   Moreover, the purpose of the c-source metadata module is to provide an 
global entity for all DSOExportable modules a place to hold global data of the 
model. Initially its the function registry which @areusch describes 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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to