tqchen commented on a change in pull request #7152:
URL: https://github.com/apache/tvm/pull/7152#discussion_r552918919
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -442,13 +447,17 @@ class TVMPODValue_ {
protected:
friend class TVMArgsSetter;
friend class TVMRetValue;
+ friend class TVMMovableArgValue_;
TVMPODValue_() : type_code_(kTVMNullptr) {}
- TVMPODValue_(TVMValue value, int type_code) : value_(value),
type_code_(type_code) {}
+ TVMPODValue_(TVMValue value, int type_code, const std::string&
prefix_message = "")
+ : value_(value), type_code_(type_code), prefix_message_(prefix_message)
{}
/*! \brief The value */
TVMValue value_;
/*! \brief the type code */
int type_code_;
+ /*! \brief Message to prefix any error message with */
+ std::string prefix_message_;
Review comment:
I understand what this code does, want to point out the overhead in the
current approach is too large that we might want to think about alternatives :)
The efficiency and clean part of the packedfunc mechanism really matters
here that we might not want to include the feature because of the eager
construction overhead.
I am not suggesting the FFI catching is the only way, but I do think more
thoughts need to put into the process, for example, to have the error message
report the i-th argument when things went wrong, while catching will show the
name of the function.
----------------------------------------------------------------
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]
