zxybazh commented on code in PR #16039:
URL: https://github.com/apache/tvm/pull/16039#discussion_r1382777070
##########
include/tvm/target/tag.h:
##########
@@ -104,6 +104,12 @@ class TargetTagRegEntry {
* \param config The config dict for target creation
*/
inline TargetTagRegEntry& set_config(Map<String, ObjectRef> config);
+ /*!
+ * \brief Add a key-value pair to the config dict
+ * \param key The attribute name
+ * \param value The attribute value
+ */
+ inline TargetTagRegEntry& add_config(String key, ObjectRef value);
Review Comment:
`Add_config` seems to assume the key was not originally in the
configuration, what about`with_config` which isn't related to whether the
original config has the given key.
##########
src/target/target_kind.cc:
##########
@@ -334,6 +334,7 @@ TVM_REGISTER_TARGET_KIND("cuda", kDLCUDA)
.add_attr_option<Integer>("max_threads_per_block")
.add_attr_option<Integer>("thread_warp_size", Integer(32))
.add_attr_option<Integer>("registers_per_block")
+ .add_attr_option<Integer>("l2_cache_size_bytes")
Review Comment:
Nit, naming convention. Previous attributes like
`max_shared_memory_per_block` is also using bytes, what about we stick to the
same style and use `l2_cache_size`? Please feel free to add a comment to
clarify it's in bytes though.
##########
include/tvm/target/tag.h:
##########
@@ -131,6 +137,11 @@ inline TargetTagRegEntry&
TargetTagRegEntry::set_config(Map<String, ObjectRef> c
return *this;
}
+inline TargetTagRegEntry& TargetTagRegEntry::add_config(String key, ObjectRef
value) {
+ tag_->config.Set(key, value);
Review Comment:
Would you please also add the case when the key is already set?
--
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]
