huajsj commented on a change in pull request #8497:
URL: https://github.com/apache/tvm/pull/8497#discussion_r681415394
##########
File path: src/runtime/graph_executor/graph_executor.cc
##########
@@ -384,9 +446,15 @@ void GraphExecutor::SetupOpExecs() {
for (size_t i = 0; i < inode.inputs.size(); i++) {
uint32_t eid = this->entry_id(inode.inputs[i]);
- // check if op input is model input
- if (input_node_eids.count(eid) > 0) {
-
input_dltensors_[eid].push_back(static_cast<DLTensor*>(op_args->arg_values[i].v_handle));
Review comment:
if the purpose is to modify the input node "connect to the output node"
then here code should do same thing as logic asked instead of push "all node"
into input_dltensors that conflict with the logic.
this logic seems like not cause a serious bug, but first it will cause
confuse issue like "why all input get push into input_dltensors_ but only
partial get used that will make maintain become difficult, secondly this
unnecessary
logic that tracking all node input increased the performance cost for all
existing use case of GraphExecutor especially for big network case, I think
here should fix such logic issue and avoid these side effect.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
