tqchen commented on a change in pull request #7152:
URL: https://github.com/apache/tvm/pull/7152#discussion_r556737215
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -562,6 +570,37 @@ class TVMMovableArgValue_ : public TVMPODValue_ {
TVMArgValue AsArgValue() const { return TVMArgValue(value_, type_code_); }
};
+/*!
+ * \brief Internal auxiliary struct for TypedPackedFunc to indicate a movable
argument
+ * with additional context information for better error reporting.
+ *
+ * \sa MovableArgValue_
+ * \note For internal development purpose only.
+ */
+class TVMMovableArgValueWithContext_ {
+ public:
+ public:
+ TVMMovableArgValueWithContext_(TVMValue value, int type_code, int arg_index,
+ const std::string* name)
+ : value_(value, type_code), arg_index_(arg_index), name_(name) {}
+
+ template <typename T>
+ inline operator T() const {
Review comment:
when the function is inside a class body, iline is not needed
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -562,6 +570,37 @@ class TVMMovableArgValue_ : public TVMPODValue_ {
TVMArgValue AsArgValue() const { return TVMArgValue(value_, type_code_); }
};
+/*!
+ * \brief Internal auxiliary struct for TypedPackedFunc to indicate a movable
argument
+ * with additional context information for better error reporting.
+ *
+ * \sa MovableArgValue_
+ * \note For internal development purpose only.
+ */
+class TVMMovableArgValueWithContext_ {
+ public:
+ public:
Review comment:
two public, document the arguments
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -562,6 +570,37 @@ class TVMMovableArgValue_ : public TVMPODValue_ {
TVMArgValue AsArgValue() const { return TVMArgValue(value_, type_code_); }
};
+/*!
+ * \brief Internal auxiliary struct for TypedPackedFunc to indicate a movable
argument
+ * with additional context information for better error reporting.
+ *
+ * \sa MovableArgValue_
+ * \note For internal development purpose only.
+ */
+class TVMMovableArgValueWithContext_ {
+ public:
+ public:
+ TVMMovableArgValueWithContext_(TVMValue value, int type_code, int arg_index,
+ const std::string* name)
+ : value_(value, type_code), arg_index_(arg_index), name_(name) {}
+
+ template <typename T>
+ inline operator T() const {
+ try {
+ return value_;
+ } catch (dmlc::Error& e) {
+ LOG(FATAL) << "In function " << (name_ == nullptr ? "<anonymous>" :
*name_)
+ << ": error while converting argument " << arg_index_ << ": "
<< e.what();
+ throw "";
+ }
+ }
+
+ private:
+ TVMMovableArgValue_ value_;
+ int arg_index_;
+ const std::string* name_;
Review comment:
rename to opt_name_ indicate this is optional
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -1297,14 +1339,18 @@ TypedPackedFunc<R(Args...)>::TypedPackedFunc(const
TVMArgValue& value)
: packed_(value.operator PackedFunc()) {}
template <typename R, typename... Args>
-TypedPackedFunc<R(Args...)>::TypedPackedFunc(TVMMovableArgValue_&& value)
+TypedPackedFunc<R(Args...)>::TypedPackedFunc(TVMMovableArgValueWithContext_&&
value)
: packed_(value.operator PackedFunc()) {}
template <typename R, typename... Args>
template <typename FType>
-inline void TypedPackedFunc<R(Args...)>::AssignTypedLambda(FType flambda) {
- packed_ = PackedFunc([flambda](const TVMArgs& args, TVMRetValue* rv) {
Review comment:
Let us keep an implementation when name is not provided, and no
capturing is needed
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -1297,14 +1339,18 @@ TypedPackedFunc<R(Args...)>::TypedPackedFunc(const
TVMArgValue& value)
: packed_(value.operator PackedFunc()) {}
template <typename R, typename... Args>
-TypedPackedFunc<R(Args...)>::TypedPackedFunc(TVMMovableArgValue_&& value)
+TypedPackedFunc<R(Args...)>::TypedPackedFunc(TVMMovableArgValueWithContext_&&
value)
: packed_(value.operator PackedFunc()) {}
template <typename R, typename... Args>
template <typename FType>
-inline void TypedPackedFunc<R(Args...)>::AssignTypedLambda(FType flambda) {
- packed_ = PackedFunc([flambda](const TVMArgs& args, TVMRetValue* rv) {
- detail::unpack_call<R, sizeof...(Args)>(flambda, args, rv);
+inline void TypedPackedFunc<R(Args...)>::AssignTypedLambda(FType flambda,
std::string name) {
+ packed_ = PackedFunc([flambda, name](const TVMArgs& args, TVMRetValue* rv) {
+ if (args.size() != sizeof...(Args)) {
+ LOG(FATAL) << "Function " << (name.size() == 0 ? "<anonymous>" : name)
<< " expects "
Review comment:
in this particular name is provided, and we can always print a name
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -336,8 +337,11 @@ class TVMArgs {
inline const char* ArgTypeCode2Str(int type_code);
// macro to check type code.
+#define TVM_ICHECK_TYPE_CODE(CODE, T) \
+ ICHECK_EQ(CODE, T) << " expected " << ArgTypeCode2Str(T) << " but got " <<
ArgTypeCode2Str(CODE)
Review comment:
Given the macro is only used inside the packed_func, let us only keep
one version. TVM_CHECK_TYPE_CODE should be fine for this case. (feel free to
use ICHECK internally)
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -562,6 +570,37 @@ class TVMMovableArgValue_ : public TVMPODValue_ {
TVMArgValue AsArgValue() const { return TVMArgValue(value_, type_code_); }
};
+/*!
+ * \brief Internal auxiliary struct for TypedPackedFunc to indicate a movable
argument
+ * with additional context information for better error reporting.
+ *
Review comment:
additional context information such as function name and argument index
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -526,7 +533,8 @@ class TVMArgValue : public TVMPODValue_ {
*/
class TVMMovableArgValue_ : public TVMPODValue_ {
public:
- TVMMovableArgValue_(TVMValue value, int type_code) : TVMPODValue_(value,
type_code) {}
+ TVMMovableArgValue_(TVMValue value, int type_code, const std::string& name =
"", int argnum = -1)
Review comment:
this change is no longer needed
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -562,6 +570,37 @@ class TVMMovableArgValue_ : public TVMPODValue_ {
TVMArgValue AsArgValue() const { return TVMArgValue(value_, type_code_); }
};
+/*!
+ * \brief Internal auxiliary struct for TypedPackedFunc to indicate a movable
argument
+ * with additional context information for better error reporting.
+ *
+ * \sa MovableArgValue_
+ * \note For internal development purpose only.
+ */
+class TVMMovableArgValueWithContext_ {
+ public:
+ public:
+ TVMMovableArgValueWithContext_(TVMValue value, int type_code, int arg_index,
+ const std::string* name)
+ : value_(value, type_code), arg_index_(arg_index), name_(name) {}
+
+ template <typename T>
+ inline operator T() const {
+ try {
+ return value_;
+ } catch (dmlc::Error& e) {
+ LOG(FATAL) << "In function " << (name_ == nullptr ? "<anonymous>" :
*name_)
+ << ": error while converting argument " << arg_index_ << ": "
<< e.what();
+ throw "";
Review comment:
add a comment about throw
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -1213,21 +1252,22 @@ namespace detail {
template <typename R, int nleft, int index, typename F>
struct unpack_call_dispatcher {
template <typename... Args>
- TVM_ALWAYS_INLINE static void run(const F& f, const TVMArgs& args_pack,
TVMRetValue* rv,
- Args&&... unpacked_args) {
+ TVM_ALWAYS_INLINE static void run(const std::string* name, const F& f, const
TVMArgs& args_pack,
+ TVMRetValue* rv, Args&&... unpacked_args) {
// construct a movable argument value
// which allows potential move of argument to the input of F.
unpack_call_dispatcher<R, nleft - 1, index + 1, F>::run(
- f, args_pack, rv, std::forward<Args>(unpacked_args)...,
- TVMMovableArgValue_(args_pack.values[index],
args_pack.type_codes[index]));
+ name, f, args_pack, rv, std::forward<Args>(unpacked_args)...,
+ TVMMovableArgValueWithContext_(args_pack.values[index],
args_pack.type_codes[index], index,
+ name));
}
};
template <typename R, int index, typename F>
struct unpack_call_dispatcher<R, 0, index, F> {
template <typename... Args>
- TVM_ALWAYS_INLINE static void run(const F& f, const TVMArgs& args_pack,
TVMRetValue* rv,
- Args&&... unpacked_args) {
+ TVM_ALWAYS_INLINE static void run(const std::string* name, const F& f, const
TVMArgs& args_pack,
Review comment:
name -> optional_name
----------------------------------------------------------------
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]
