[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6697: [BYOC] Allow custom codegens to register their own constant updater

[GitBox]

mbaret commented on a change in pull request #6697:
URL: https://github.com/apache/incubator-tvm/pull/6697#discussion_r514351237



##########
File path: src/relay/backend/graph_runtime_codegen.cc
##########
@@ -371,10 +371,28 @@ class GraphRuntimeCodegen : public 
backend::MemoizedExprTranslator<std::vector<G
 
       // Step into the functions that are handled by external codegen to
       // collect metadata.
+      auto codegen = func->GetAttr<String>(attr::kCompiler);
+      CHECK(codegen.defined()) << "No external codegen is set";
+      std::string codegen_name = codegen.value();
       const auto name_node = func->GetAttr<String>(tvm::attr::kGlobalSymbol);
-      std::string symobl = std::string(name_node.value());
-      ConstantUpdater const_visit(symobl, &params_);
-      const_visit(func);
+      std::string symbol = std::string(name_node.value());
+      std::string const_update_name = "relay.ext." + codegen_name + 
".constant_updater";

Review comment:
       Agreed, but there's about 20 shared lines in this change between the 
two, not just those two. That's why I suggested creating a shared utility 
function rather than moving it all into ConstantUpdater.




----------------------------------------------------------------
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:
us...@infra.apache.org