tqchen commented on a change in pull request #8750:
URL: https://github.com/apache/tvm/pull/8750#discussion_r689492820
##########
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, based on the considerations:
- C0: It will remove one level of indirection(f->GetAttr vs
f->attrs.GetAttr) and gives more clear documentation(since developer usually
looks up doc on the Function or IRModule themselves)
- 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]
