-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TVMC] Compose target options from target registry #9218
Conversation
The `test_tvmc_common.py` file was becoming a bit of a mixed bag of tests and as we now want to extend the `Target` processing logic it made sense to split each out into its own file to make it clearer what each does. `test_common.py` has also been renamed before we start using it for all the tests instead.
[The RFC for this is still under discussion](https://github.com/apache/tvm-rfcs/pulls), but doing this before splitting the registries makes the most sense. This enables the `tvmc` driver to re-combobulate Target options from arguments: ``` tvmc --target=llvm \ --target-llvm-mcpu=cortex-m3 ```
5bcae99
to
9c07311
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will allow others to comment as well
@@ -0,0 +1,74 @@ | |||
# Licensed to the Apache Software Foundation (ASF) under one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should target_from_cli move here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yip, I didn't realise test_common.py
matched with common.py
, so there'll be another PR at some point to split out the mixed set of things in common.py
.
# We can't tell the type inside an Array but all current options are strings so | ||
# it can default to that. Bool is used alongside Integer but aren't distinguished | ||
# between as both are represented by IntImm | ||
INTERNAL_TO_NATIVE_TYPE = {"runtime.String": str, "IntImm": int, "Array": str} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is kind of unfortunate but i see there isn't really much way around it right now without a very large refactor.
* [TVMC] Split common tvmc test file into more specific files The `test_tvmc_common.py` file was becoming a bit of a mixed bag of tests and as we now want to extend the `Target` processing logic it made sense to split each out into its own file to make it clearer what each does. `test_common.py` has also been renamed before we start using it for all the tests instead. * [TVMC] Compose target options from target registry [The RFC for this is still under discussion](https://github.com/apache/tvm-rfcs/pulls), but doing this before splitting the registries makes the most sense. This enables the `tvmc` driver to re-combobulate Target options from arguments: ``` tvmc --target=llvm \ --target-llvm-mcpu=cortex-m3 ```
* [TVMC] Split common tvmc test file into more specific files The `test_tvmc_common.py` file was becoming a bit of a mixed bag of tests and as we now want to extend the `Target` processing logic it made sense to split each out into its own file to make it clearer what each does. `test_common.py` has also been renamed before we start using it for all the tests instead. * [TVMC] Compose target options from target registry [The RFC for this is still under discussion](https://github.com/apache/tvm-rfcs/pulls), but doing this before splitting the registries makes the most sense. This enables the `tvmc` driver to re-combobulate Target options from arguments: ``` tvmc --target=llvm \ --target-llvm-mcpu=cortex-m3 ```
* [TVMC] Split common tvmc test file into more specific files The `test_tvmc_common.py` file was becoming a bit of a mixed bag of tests and as we now want to extend the `Target` processing logic it made sense to split each out into its own file to make it clearer what each does. `test_common.py` has also been renamed before we start using it for all the tests instead. * [TVMC] Compose target options from target registry [The RFC for this is still under discussion](https://github.com/apache/tvm-rfcs/pulls), but doing this before splitting the registries makes the most sense. This enables the `tvmc` driver to re-combobulate Target options from arguments: ``` tvmc --target=llvm \ --target-llvm-mcpu=cortex-m3 ```
The RFC for this is still under discussion, but doing this before splitting the registries makes the most sense. This enables the
tvmc
driver to re-combobulate Target options from arguments:This rolls in #9206 so you may want to just look at the second commit.