[GitHub] [tvm] giuseros commented on a change in pull request #7988: AOT] Remove lookup parameter function in AOT

[GitBox <gi...@apache.org>]

giuseros commented on a change in pull request #7988:
URL: https://github.com/apache/tvm/pull/7988#discussion_r627561948



##########
File path: src/target/source/codegen_c.cc
##########
@@ -662,6 +662,11 @@ void CodeGenC::VisitExpr_(const CallNode* op, std::ostream& os) {  // NOLINT(*)
       os << " != ";
       this->PrintExpr(op->args[0], os);
       os << ")";
+    } else if (op->op.same_as(builtin::lookup_param())) {
+      ICHECK_EQ(op->args.size(), 1);
+      const StringImmNode* str = op->args[0].as<StringImmNode>();
+      ICHECK(str != nullptr);
+      os << "__tvm_param__" << str->value;

Review comment:
       Hi Andrew, thanks for the comment, but I am not sure I follow. When using `--link-params`, how do we change parameters at runtime in AOT as of now? AFAIK the reason for that function to exist was to let the graph executor to get the params during the execution. AOT does not need this functionality, so we can remove the function, exactly how we removed the function registry. In the future, we may want to add a feature to change those parameters at runtime. I can do an RFC, but this change seems very small and straightforward. Just to be clear, I am not adding/removing any functionality, I am only making aot faster and smaller. 




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