You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/01/20 11:37:51 UTC
[GitHub] [incubator-tvm] FrozenGene opened a new pull request #4749:
[AutoTVM] Support to pass additional compiler options
FrozenGene opened a new pull request #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749
Currently, AutoTVM doesn't support to pass additional compiler options to builder. It works fine on most cases. However, if we use NDK CC, some times we want to specify additional compiler options. For example, another `sysroot`.
After this PR, the user could do like this:
```python
os.environ['TVM_NDK_CC'] = /path/to/ndk_cc
options=['--sysroot=/path/to/sysroot', '-shared', '-fPIC']
tuning_option = {
// ...
'measure_option': autotvm.measure_option(
builder=autotvm.LocalBuilder(
build_func='ndk' if use_android else 'default', options=options),
...
),
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582194982
> Try something along the following line:
>
> ```python
> from tvm.contrib import cc, ndk
>
> build_func = cc.cross_compiler(ndk.create_shared, options)
>
> tuning_option = {
> // ...
> 'measure_option': autotvm.measure_option(
> builder=autotvm.LocalBuilder(
> build_func=build_func if use_android else 'default', options=options),
> ...
> ),
> }
> ```
>
> Perhaps it is a good time to document these usecases of cross_compiler(e.g. get target triple from compile_func if it has the attr)
Yes. Your way is good and don't change anything. Do you mean this ?
```python
from tvm.contrib import cc, ndk
build_func = cc.cross_compiler(ndk.create_shared, options)
tuning_option = {
// ...
'measure_option': autotvm.measure_option(
builder=autotvm.LocalBuilder(
build_func=build_func if use_android else 'default'),
...
),
}
```
No options needed in the builder.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy edited a comment on issue #4749:
[AutoTVM] Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
merrymercy edited a comment on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582055130
1. We should not put `options` in `MeasureResult`. It is not the result (output). It is input.
2. If you change MeasureInput / MeasureResult, you should also update serialization for them in `python/tvm/autotvm/record.py::encode, decode`. You should be very careful about the backward-compatibility.
3. If you add `options` to records, why don't you also add `build_func`?
Currently, I think not serializing them is fine. Or we can attach `options` into `MeasureInput.task` and serialize them carefully.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4749: [AutoTVM] Support
to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582190081
Try this:
```python
from tvm.contrib import cc, ndk
build_func = cc.cross_compiler(
ndk.create_shared, options)
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy edited a comment on issue #4749:
[AutoTVM] Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
merrymercy edited a comment on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582055130
1. We should not put `options` in `MeasureResult`. It is not the result (output). It is input.
2. If you change MeasureInput / MeasureResult, you should also update serialization for them in `python/tvm/autotvm/record.py::encode, decode`. You should be very careful about the backward-compatibility.
3. If you add `options` to records, why don't you also add `build_func`?
Currently, I think not serializing them is fine. Or we can embed `options` into `MeasureInput.task` and serialize them carefully.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582195632
> 1. We should not put `options` in `MeasureResult`. It is not the result (output). It is input.
> 2. If you change MeasureInput / MeasureResult, you should also update serialization for them in `python/tvm/autotvm/record.py::encode, decode`. You should be very careful about the backward-compatibility.
> 3. If you add `options` to records, why don't you also add `build_func`?
>
> Currently, I think not serializing them is fine. Or we can attach `options` into `MeasureInput.task` and serialize them carefully.
Yeah. I agree with you. I think this PR is not one elegant solution for this problem. The nicer way is @tqchen 's suggestion, using custom build func. I will close this PR. Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen edited a comment on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582188780
Likely you can create a build function(via functor that captures these additional options) instead of directly passing ndk.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4749: [AutoTVM] Support
to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582188780
Likely you can create a build function(via functor that captures these additional options) instead of directly passing ndk
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen edited a comment on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582190081
Try something along the following line:
```python
from tvm.contrib import cc, ndk
build_func = cc.cross_compiler(ndk.create_shared, options)
tuning_option = {
// ...
'measure_option': autotvm.measure_option(
builder=autotvm.LocalBuilder(
build_func=build_func if use_android else 'default', options=options),
...
),
}
```
Perhaps it is a good time to document these usecases of cross_compiler(e.g. get target triple from compile_func if it has the attr)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] comaniac commented on a change in pull request
#4749: [AutoTVM] Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#discussion_r368778639
##########
File path: python/tvm/module.py
##########
@@ -162,13 +162,20 @@ def export_library(self,
f.write(_PackImportsToC(self, is_system_lib))
files.append(path_cc)
- if has_c_module:
- options = []
- if "options" in kwargs:
- opts = kwargs["options"]
- options = opts if isinstance(opts, (list, tuple)) else [opts]
- opts = options + ["-I" + path for path in find_include_path()]
- kwargs.update({'options': opts})
+ # Make sure we won't pass {'options': None} or {'options': [None, '-std=c++11', ...]}
+ # to compiler. We can not prevent users doing the following code:
+ # f.export_library(path_dso, cc.create_shared, options=None)
+ # f.export_library(path_dso, cc.create_shared, options=[None, '-std=c++11'])
+ if "options" in kwargs or has_c_module:
+ opts = kwargs["options"] if "options" in kwargs else None
+ options = opts if isinstance(opts, (list, tuple)) else [opts]
+ options = [opt for opt in options if opt]
+ if has_c_module:
+ options = options + ["-I" + path for path in find_include_path()]
+ if not options:
+ del kwargs["options"]
+ else:
+ kwargs.update({'options': options})
Review comment:
I feel like we don't have to test `has_c_module` twice. Maybe the following logic is sufficient?
```
options = ["-I" +path for path in find_include_path()] if has_c_module else []
if "options" in kwargs:
opts = kwargs["options"]
options += opts if isinstance(opts, (list, tuple)) else [opts]
options = [opt for opt in options if opt]
if not options:
del kwargs["options"]
if options:
kwargs.update({"options": options})
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen edited a comment on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582190081
Try something along the following line:
```python
from tvm.contrib import cc, ndk
build_func = cc.cross_compiler(ndk.create_shared, options)
tuning_option = {
// ...
'measure_option': autotvm.measure_option(
builder=autotvm.LocalBuilder(
build_func=build_func if use_android else 'default', options=options),
...
),
}
```
Perhaps it is a good time to document these usecases of cross_compiler(e.g. get target triple from compile_func if it has the attr)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request
#4749: [AutoTVM] Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#discussion_r374597354
##########
File path: python/tvm/module.py
##########
@@ -162,13 +162,20 @@ def export_library(self,
f.write(_PackImportsToC(self, is_system_lib))
files.append(path_cc)
- if has_c_module:
- options = []
- if "options" in kwargs:
- opts = kwargs["options"]
- options = opts if isinstance(opts, (list, tuple)) else [opts]
- opts = options + ["-I" + path for path in find_include_path()]
- kwargs.update({'options': opts})
+ # Make sure we won't pass {'options': None} or {'options': [None, '-std=c++11', ...]}
+ # to compiler. We can not prevent users doing the following code:
+ # f.export_library(path_dso, cc.create_shared, options=None)
+ # f.export_library(path_dso, cc.create_shared, options=[None, '-std=c++11'])
+ if "options" in kwargs or has_c_module:
+ opts = kwargs["options"] if "options" in kwargs else None
+ options = opts if isinstance(opts, (list, tuple)) else [opts]
+ options = [opt for opt in options if opt]
+ if has_c_module:
+ options = options + ["-I" + path for path in find_include_path()]
+ if not options:
+ del kwargs["options"]
+ else:
+ kwargs.update({'options': options})
Review comment:
Done. Please review it again.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request
#4749: [AutoTVM] Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#discussion_r373905750
##########
File path: python/tvm/module.py
##########
@@ -162,13 +162,20 @@ def export_library(self,
f.write(_PackImportsToC(self, is_system_lib))
files.append(path_cc)
- if has_c_module:
- options = []
- if "options" in kwargs:
- opts = kwargs["options"]
- options = opts if isinstance(opts, (list, tuple)) else [opts]
- opts = options + ["-I" + path for path in find_include_path()]
- kwargs.update({'options': opts})
+ # Make sure we won't pass {'options': None} or {'options': [None, '-std=c++11', ...]}
+ # to compiler. We can not prevent users doing the following code:
+ # f.export_library(path_dso, cc.create_shared, options=None)
+ # f.export_library(path_dso, cc.create_shared, options=[None, '-std=c++11'])
+ if "options" in kwargs or has_c_module:
+ opts = kwargs["options"] if "options" in kwargs else None
+ options = opts if isinstance(opts, (list, tuple)) else [opts]
+ options = [opt for opt in options if opt]
+ if has_c_module:
+ options = options + ["-I" + path for path in find_include_path()]
+ if not options:
+ del kwargs["options"]
+ else:
+ kwargs.update({'options': options})
Review comment:
Yes. Your way could avoid twice check of `has_c_module`.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy closed pull request #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
merrymercy closed pull request #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on issue #4749: [AutoTVM]
Support to pass additional compiler options
Posted by GitBox <gi...@apache.org>.
merrymercy commented on issue #4749: [AutoTVM] Support to pass additional compiler options
URL: https://github.com/apache/incubator-tvm/pull/4749#issuecomment-582055130
1. We should not put `options` in `MeasureResult`. It is not result (output). It is input.
2. If you change MeasureInput / MeasureResult, you should also update serialization for them in `python/tvm/autotvm/record.py::encode, decode`. You should be very careful about the backward-compatibility.
3. If you add `options` to records, why don't you also add `build_func`?
Currently, I think not serializing them is fine. Or we can embed `options` into `MeasureInput.task` and serialize them carefully.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services