areusch commented on a change in pull request #7988:
URL: https://github.com/apache/tvm/pull/7988#discussion_r627594551
##########
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:
right now GraphExecutor calls `lookup_linked_param` to get a reference
to each parameter, then stores that pointer locally in GraphExecutor. the
stored pointer is what's actually used in computation.
i agree we can focus in the short term on making AOT work. my question is
basically: if we go the step further and push those parameters into operator
functions, we'll lose the ability to override them without adding some
additional argument to each operator function. so just wanted to ask whether
this was a direction you wanted to go later on?
otherwise, I don't think we need an RFC for this change standalone.
--
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]
