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



##########
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:
       i think my main question with this is: how can we support a user 
overriding a param at runtime? or, switching to a different "set" of 
parameters? we don't have to solve that with this PR, but it seems like we need 
to retain some level of indirection. given this is a fairly infrequent, 
non-deployment use case, it seems like we could maintain a single global 
pointer to a "parameter struct," and that struct would contain pointers to 
various params used by the model. user code could then override that.
   
   so my main question here is: having such a struct implies a function 
argument or global pointer state. does that agree with the direction you want 
to take this? perhaps if this PR is part of something larger than just "remove 
an unnecessary function call," could you sketch a short (like 2-3 paragraph) 
RFC describing what you want to do?




-- 
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