tqchen commented on a change in pull request #8750:
URL: https://github.com/apache/tvm/pull/8750#discussion_r689494759
##########
File path: include/tvm/target/target.h
##########
@@ -54,7 +54,7 @@ class TargetNode : public Object {
/*! \brief Keys for this target */
Array<String> keys;
/*! \brief Collection of attributes */
- Map<String, ObjectRef> attrs;
+ Map<String, ObjectRef> attrs; // TODO(@electriclilies): Unify with
DictAttrs on IRModule
Review comment:
Target attrs is different from the IRModule attrs. They contain
attributes about a target(cuda) that can be shared across IRModule or functions.
Please double check if we really want to remove the target.attrs(my guess is
that it is unlikely) would be great to confirm with the original authors of the
target (e.g. @zxybazh )
##########
File path: include/tvm/ir/attrs.h
##########
@@ -232,6 +233,72 @@ class DictAttrs : public Attrs {
*/
TVM_DLL explicit DictAttrs(Map<String, ObjectRef> dict);
+ // Utils for accessing attributes
+ // This needs to be on DictAttrs, not DictAttrsNode because we return the
default
+ // value if DictAttrsNode is not defined.
+ /*!
+ * \brief Get a function attribute.
+ *
+ * \param attr_key The attribute key.
+ * \param default_value The default value if the key does not exist,
defaults to nullptr.
+ *
+ * \return The result
+ *
+ * \tparam TOBjectRef the expected object type.
+ * \throw Error if the key exists but the value does not match TObjectRef
+ *
+ * \code
+ *
+ * void GetAttrExample(const BaseFunc& f) {
+ * auto value = f->attrs.GetAttr<Integer>("AttrKey", 0);
Review comment:
would be great to keep the function GetAttr and HasNonZeroAttr function
in BaseFunc and IRModule. For two considerations:
- C0: It will remove one level of indirection(f->GetAttr vs
f->attrs.GetAttr) and gives more clear documentation
- C1: API consitency: WithAttr directly operates on the function and module,
and the functions with related functionalities should ideally be made
consistent with this usage.
- C2: If there is a future refactor that changes DictAttr => Map, the API
can be made consistent in a backward compaitble way
We can of course keep the common impl as well and redirect in the function
and module case.
--
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]
