comaniac commented on a change in pull request #6369:
URL: https://github.com/apache/incubator-tvm/pull/6369#discussion_r485768863
##########
File path: include/tvm/target/target.h
##########
@@ -50,7 +50,11 @@ class TargetNode : public Object {
Array<String> keys;
/*! \brief Collection of attributes */
Map<String, ObjectRef> attrs;
- /*! \return the full device string to pass to codegen::Build */
+ /*!
+ * \brief The raw string representation of the target
+ * \return the full device string to pass to codegen::Build
+ * \note It will be deprecated after the Target RFC is fulled landed.
Review comment:
s/fulled/fully/
##########
File path: python/tvm/target/tag.py
##########
@@ -28,7 +28,9 @@ def list_tags() -> Dict[str, Target]:
tag_dict : Dict[str, Target]
The dict of tags mapping each tag name to to its corresponding target
"""
- return _ffi_api.TargetTagListTags()
+ if hasattr(_ffi_api, "TargetTagListTags"):
+ return _ffi_api.TargetTagListTags()
+ return None
Review comment:
According to the return type, here should be `{}`; otherwise the type
should be `Optional[Dict[str, Target]]`. Either way is fine.
##########
File path: python/tvm/target/tag.py
##########
@@ -41,7 +43,8 @@ def register_tag(name: str, config: Dict[str, Any], override:
bool = False) -> T
config : Dict[str, Any]
The config dict used to create the target
override: bool
- A boolean flag indicating if overriding existing tags are allowed
+ A boolean flag indicating if overriding existing tags are allowed.
+ If the flag is False, an exception will be throw when the tag has been
added previously
Review comment:
```suggestion
If False and the tag has been registered already, an exception will
be thrown.
```
##########
File path: python/tvm/target/tag.py
##########
@@ -57,14 +60,14 @@ def register_tag(name: str, config: Dict[str, Any],
override: bool = False) -> T
"arch": "sm_61",
})
"""
- return _ffi_api.TargetTagAddTag(name, config, override)
+ if hasattr(_ffi_api, "TargetTagAddTag"):
+ return _ffi_api.TargetTagAddTag(name, config, override)
+ return None
-# We do a round of tag listing on loading time to verify all tag information
-if hasattr(_ffi_api, "TargetTagAddTag"):
- list_tags()
+list_tags()
Review comment:
Per offline discussion, the purpose of making this call is to check the
tag correctness. Based on this, it would be better to make an explicit
description in `list_tags()`, saying that this function also performs
correctness checking.
##########
File path: python/tvm/target/tag.py
##########
@@ -57,14 +60,14 @@ def register_tag(name: str, config: Dict[str, Any],
override: bool = False) -> T
"arch": "sm_61",
})
"""
- return _ffi_api.TargetTagAddTag(name, config, override)
+ if hasattr(_ffi_api, "TargetTagAddTag"):
+ return _ffi_api.TargetTagAddTag(name, config, override)
+ return None
Review comment:
ditto. `Optional[Target]`.
##########
File path: python/tvm/target/target.py
##########
@@ -399,8 +399,8 @@ def create_llvm(llvm_args):
def create(target):
"""Deprecated. Use the constructor of :py:mod:`tvm.target.Target` directly.
"""
- warnings.warn('tvm.target.create() is going to be deprecated. '
- 'Please use the constructor of tvm.target.Target directly')
+ warnings.warn(
+ 'tvm.target.create() is being deprecated. Please tvm.target.Target()
instead')
Review comment:
```suggestion
'tvm.target.create() is being deprecated. Please use
tvm.target.Target() instead')
```
##########
File path: python/tvm/target/tag.py
##########
@@ -57,14 +60,14 @@ def register_tag(name: str, config: Dict[str, Any],
override: bool = False) -> T
"arch": "sm_61",
})
"""
- return _ffi_api.TargetTagAddTag(name, config, override)
+ if hasattr(_ffi_api, "TargetTagAddTag"):
+ return _ffi_api.TargetTagAddTag(name, config, override)
+ return None
-# We do a round of tag listing on loading time to verify all tag information
-if hasattr(_ffi_api, "TargetTagAddTag"):
- list_tags()
+list_tags()
- register_tag("nvidia/gtx1080ti", config={
- "kind": "cuda",
- "arch": "sm_61",
- })
+register_tag("nvidia/gtx1080ti", config={
+ "kind": "cuda",
+ "arch": "sm_61",
+})
Review comment:
Per offline discussion, we maintain all tags in the C++ side to support
pure C++ use cases. The Python API is only used for prototyping.
----------------------------------------------------------------
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]
