leandron commented on a change in pull request #7304:
URL: https://github.com/apache/tvm/pull/7304#discussion_r576385954
##########
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:
I feel the JSON support in general from TVMC is something that we should
review. The JSON is something we should be hiding from users in the command
line, and be using internally as an API protocol.
The inline format as in `--target={ json_formatted_string }` is not
something end-users really need IMHO. The `<path to json>` requires a lot of
internal knowledge about how the internal APIs work, so it is a very advanced
developer option, on a format not really documented AFAIK for end-users.
So, after a lot of thinking in this one, I think we have two options:
a. We can keep JSON support, to talk directly to the Target API, and hence
only supporting what the API supports (it would be something like a dev.
option), or
b. We hide the JSON support from the command-line end-user (removing it on a
separate PR), and when support for composite targets appears from an API point
of view, we keep the user-friendly syntax from the command line, and
under-the-hood generate the appropriate JSON to fulfill what the user asked.
So, as a plan, I suggest we go with `a` for now, and open a broader
discussion to potentially pursue `b` if there is agreements.
----------------------------------------------------------------
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]
