leandron commented on a change in pull request #7304:
URL: https://github.com/apache/tvm/pull/7304#discussion_r577043131
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -91,18 +272,37 @@ def target_from_cli(target):
-------
tvm.target.Target
an instance of target device information
+ codegens : list of dict
+ This list preserves the order in which codegens were
+ provided via command line. Each Dict contains three keys:
+ 'kind', containing the name of the codegen; 'opts' containing
+ a key-value for all options passed via CLI; 'raw',
+ containing the plain string for this codegen
"""
+ extra_codegens = []
if os.path.exists(target):
with open(target) as target_file:
- logger.info("using target input from file: %s", target)
+ logger.info("target input is a path: %s", target)
target = "".join(target_file.readlines())
+ elif is_inline_json(target):
+ logger.info("target input is inline JSON: %s", target)
+ else:
+ logger.info("target input is plain text: %s", target)
+ try:
+ parsed_targets = parse_target(target)
+ except ValueError as ex:
+ raise TVMCException(f"Error parsing target string '{target}'.\nThe
error was: {ex}")
+
+ validate_targets(parsed_targets)
+ target = parsed_targets[-1]["raw"]
+ extra_codegens = parsed_targets[:-1] if len(parsed_targets) > 1 else []
# TODO(@leandron) We don't have an API to collect a list of supported
# targets yet
logger.debug("creating target from input: %s", target)
- return tvm.target.Target(target)
+ return tvm.target.Target(target), extra_codegens
Review comment:
> It seems to me that the point we should discuss here is not the TVM
target system. The discussion about whether target system should use
hierarchical structure or not should be in a separate thread.
Agreed. The main point in discussion (that I tried exemplifying in my
comment above) is whether or not the decision of having the internal Target API
to accept only _JSON_ or a _tag_ as inputs, and making that *the only way* to
specify targets on TVMC, makes the usability of the command line tool to be
good enough for the end-user.
Having the inline JSON input directly into the tool makes it overly
complicated (from the usual command-line tool user expectations). The tags
system is a good shortcut to save the hassle of typing JSON, however it lacks
the flexibility, as we can't pass parameters to them, nor change order of
targets without going to the same hassle of JSON maintenance and editing.
This is why I think we will need this translation from an inline format to
the internal JSON format that will be passed to the Target API. For now, to
support new codegens, we'll need to do that registration process, as proposed
in this PR.
> 1) the possible future discrepancy and maintenance efforts. Specifically,
do we need to change the target processing logic in TVMC frequently if there's
something changed in the target system (e.g., new attribute, new target, etc)?
In the current JSON schema, we can map all that is being proposed into an
inline sorted list of target, as string. If there are global top level "new
attributes" to the target specification, they will correspond to top level
`--new-attribute <value>` options (and will require the implementation of the
new attribute accordingly). If options are nested into the targets themselves,
they will be part of the existing `--target` option.
Specifically for new targets, once support for the partition comes from the
Target API, we can keep the current syntax and just move it be processes by the
Target API, and we'll just need to maintain the syntax-sugar for the end user.
----------------------------------------------------------------
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]
