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 2021/02/02 16:23:23 UTC
[GitHub] [tvm] cbalint13 opened a new pull request #7392: Fix rand_r() rpc proper inclusion
cbalint13 opened a new pull request #7392:
URL: https://github.com/apache/tvm/pull/7392
* The following error is encountered:
```
RuntimeError: error while running command "arm-none-eabi-g++ -mcpu=cortex-m33 -std=c++11 -Wall -Werror
...
...
/usr/lib64/python3.9/site-packages/tvm/src/runtime/crt/host/main.cc:106:18:
error: 'rand_r' was not declared in this scope; did you mean 'random'?
106 | int random = rand_r(&random_seed);
| ^~~~~~
| random
```
* Now, POSIX ```rand_r()``` is not visible using ```-std=c++11```, one have to add ```-D_GNU_SOURCE``` or lower the 2011 requirement.
---
The proposed PR fix the issue, also fixes missing propagation to ```ldflags``` (the final binary compilation).
@areusch , @manupa-arm , @mbrookhart please help with the review.
Thank you !
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772061076
> in the short term, I think it would be great to change DefaultCompiler to error when you try to compile a file with a cortex-m target. that would be adjusting the logic [here](https://github.com/apache/tvm/blob/main/python/tvm/micro/compiler.py#L101). want to take a shot at that?
@areusch ,
I will propose a protection way to avoid missusage in a separate PR.
I am closing the PR here, @manupa-arm , @areusch Lets continue on the discuss forum (right on rfc)
Thanks a lot !
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771956316
> and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
@areusch ,
First thank you for taking your time into the problem !
To not mix up things:
**A.** we could forbid (+message) if target ```micro('host')``` is used with cross/remote ```ZephyrCompiler``` & vice versa ?
**B.** what if someone don't want ```ZephyrCompiler``` but a ```DefaultCompiler``` instead (like me) ?
By ```DefaultCompiler``` i meant my own installed cross ```arm-none-eabi-g++``` in our case (which seems to be invoked at all).
By *local/generic* vs *remote/specific* terms i meant ```micro('host')``` vs ```micro(<device>)```
So,
* Should we introduce a ```CrossCompiler``` too for such purposes (as a more generic form) ?
* Then should we forbid ```local``` target to generate *remote/cross/specific* target except ```CrossCompiler``` ?
----
Would like to understand and see your reasoning on the subject first.
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772026067
@areusch yes in the case where we just want to use export_library (w/o build_static_runtime) -- as of today we can provide DefaultCompiler.library() as the fcompile if one only wants get operator lib as an .a. I mean I understand there are other ways to do that such as modifying cc.cross_compile to support archive target with using both c and c++ compilers (vs just using c++ compiler as of today).
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771956316
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772061076
> in the short term, I think it would be great to change DefaultCompiler to error when you try to compile a file with a cortex-m target. that would be adjusting the logic [here](https://github.com/apache/tvm/blob/main/python/tvm/micro/compiler.py#L101). want to take a shot at that?
@areusch ,
I will propose a protection way to avoid missusage in a separate PR.
I am closing the PR here, @manupa-arm , @areusch Lets continue on the discuss forum (right on rfc)
Thanks a lot !
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772073701
thanks @cbalint13 @manupa-arm for the great discussions. I will post up an RFC to discuss project generator API in the coming week. feel free to open topics as well if you'd like to discuss things.
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772000660
Hi @areusch ,
A follow up question, what are your thoughts on the effect of the usage of DefaultCompiler.library() function's support to get the operator library as an archive though? I can understand how this affects for DefaultCompiler.binary() or build_static_runtime (that uses DefaultCompiler.binary()). I guess my question is should we block DefaultCompiler.library() for cortex-m targets? -- or should we maybe factor that part out if we are blocking ?
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772012281
@manupa-arm are you talking about for export_library?
----------------------------------------------------------------
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
[GitHub] [tvm] areusch edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771968417
@cbalint13 it is a little bit more complicated than just specifying the Compiler implementation when cross-compiling. when using DefaultCompiler, there is assumption that all of the code needed to run the _platform_ (I.e. SoC startup code, peripheral config) lives in a `main.cc` or in a library you supply to `build_static_runtime(extra_libs=)`. on POSIX systems, this assumption doesn't impose much on the build, but in a bare metal deployment situation it's much different.
to increase compatibility with the many dev boards out there, we compile against a cross-platform deployment framework (Zephyr). most cross-platform bare-metal deployment frameworks include their own build system to determine which sources to include and the compiler config e.g. cflags, ldflags. Zephyr does this from CMakeLists.txt and prj.conf. making it convention to include this config in TVM Python scripts would mean that convention is to replicate the build system from the RTOS you're using.
unfortunately we kind of built out this Compiler abstraction without paying too much attention to that, so it's kind of the wrong level of abstraction to use here. the long-term solution to this problem is to replace Compiler with something more "project-level," where "project" roughly refers to the startup code, RTOS, and other application specific code you need to run models on a bare metal device. See #6 on the [µTVM M2 roadmap](https://discuss.tvm.apache.org/t/tvm-microtvm-m2-roadmap/8821).
to address your question B: you are still welcome to instantiate a Compiler impl of your choice and this doesn't need to live in the TVM source tree. until the project API refactor is done, happy to include additional impls into the TVM tree so long as they can be tested (see `tests/micro/qemu/test_zephyr.py`).
in the short term, I think it would be great to change DefaultCompiler to error when you try to compile a file with a cortex-m target. that would be adjusting the logic [here](https://github.com/apache/tvm/blob/main/python/tvm/micro/compiler.py#L101). want to take a shot at that?
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771968417
@cbalint13 it is a little bit more complicated than just specifying the Compiler implementation when cross-compiling. when using DefaultCompiler, there is assumption that all of the code needed to run the _platform_ (I.e. SoC startup code, peripheral config) lives in a `main.cc` or in a library you supply to `build_static_runtime(extra_libs=)`. on POSIX systems, this assumption doesn't impose much on the build, but in a bare metal deployment situation it's much different.
to increase compatibility with the many dev boards out there, we compile against a cross-platform deployment framework (Zephyr). most cross-platform bare-metal deployment frameworks include their own build system to determine which sources to include and the compiler config e.g. cflags, ldflags. Zephyr does this from CMakeLists.txt and prj.conf. making it convention to include this config in TVM Python scripts would mean that convention is to replicate the build system from the RTOS you're using.
unfortunately we kind of built out this Compiler abstraction without paying too much attention to that, so it's kind of the wrong level of abstraction to use here. the long-term solution to this problem is to replace Compiler with something more "project-level," where "project" roughly refers to the startup code, RTOS, and other application specific code you need to run models on a bare metal device. See #6 on the [µTVM M2 roadmap](https://discuss.tvm.apache.org/t/tvm-microtvm-m2-roadmap/8821).
in the short term, I think it would be great to change DefaultCompiler to error when you try to compile a file with a cortex-m target. that would be adjusting the logic [here](https://github.com/apache/tvm/blob/main/python/tvm/micro/compiler.py#L101). want to take a shot at that?
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771789962
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771776674
hey @cbalint13 can you give an example code fragment that triggers this bug? i don't think we should be compiling rand_r to bare metal--the `main.cc` present in `src/runtime/crt/host/main.cc` is intended to be compiled against a traditional POSIX OS (e.g. is used for simulating a device as a subprocess and treating stdio as the UART link).
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771809534
@cbalint13 thanks for the example. could you clarify--are you trying to run your demo as:
1. a simulation with the C runtime executing in a subprocess
2. on nrf5340dk
I ask because there are a couple of different lines that need to match:
- line 57 should be either `micro('host')` or `micro(<device>)`, depending on situation 1 or 2
- line 76 should either instantiate `DefaultCompiler` or `ZephyrCompiler`, depending on situation 1 or 2
- line 77, the argument should either be (approximately) `$CRT_ROOT_DIR/host` or `path/to/runtime/crt`, depending on situation 1 or 2
it seems like based on your comment at the end of the file, you want situation 1--so just fix up line 57 and things should work properly. right now, `DefaultCompiler` is invoking the ARM cross-compiler because it thinks you are targeting the dev board based on your target definition on line 57.
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771956316
> and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
@areusch ,
First thank you for taking your time into the problem !
To not mix up things:
**A.** we could forbid (+message) if target ```micro('host')``` is used with *cross/specific* ```ZephyrCompiler``` & vice versa ?
**B.** what if someone don't want ```ZephyrCompiler``` but a ```DefaultCompiler``` instead (like me) ?
By ```DefaultCompiler``` i meant my own installed cross ```arm-none-eabi-g++``` in our case (which seems to be invoked at all).
By *local/generic* vs *remote/specific* terms i meant ```micro('host')``` vs ```micro(<device>)```
So,
* Should we introduce a ```CrossCompiler``` too for such purposes (as a more generic form) ?
* Then should we forbid ```local``` target to generate *remote/cross/specific* target except ```CrossCompiler``` ?
----
Would like to understand and see your reasoning on the subject first.
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771956316
> and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
@areusch ,
First thank you for taking your time into the problem !
To not mix up things:
**A.** we could forbid (+message) if target ```micro('host')``` is used with *cross/specific* ```ZephyrCompiler``` & vice versa ?
**B.** what if someone don't want ```ZephyrCompiler``` (just another SDK) but a ```DefaultCompiler``` from system instead (like me) ?
By ```DefaultCompiler``` i meant my own installed cross ```arm-none-eabi-g++``` in our case (which seems to be invoked at all).
By *local/generic* vs *remote/specific* terms i meant ```micro('host')``` vs ```micro(<device>)```
So,
* Should we introduce a ```CrossCompiler``` too for such purposes (as a more generic form) ?
* Then should we forbid ```local``` target to generate *remote/cross/specific* target except ```CrossCompiler``` ?
----
Would like to understand and see your reasoning on the subject first.
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 closed pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 closed pull request #7392:
URL: https://github.com/apache/tvm/pull/7392
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 closed pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 closed pull request #7392:
URL: https://github.com/apache/tvm/pull/7392
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772000660
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771956316
> and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
@areusch ,
First thank you for taking your time into the problem !
To not mix up things:
**A.** we could forbid (+message) if target ```micro('host')``` is used with cross/remote ```ZephyrCompiler``` & vice versa ?
**B.** what if someone don't want ```ZephyrCompiler``` but a ```DefaultCompiler``` instead ?
By ```DefaultCompiler``` i meant my own installed cross ```arm-none-eabi-g++``` in our case (which seems to be invoked at all).
By *local/generic* vs *remote/specific* terms i meant ```micro('host')``` vs ```micro(<device>)```
So,
* Should we introduce a ```CrossCompiler``` too for such purposes (as a more generic form) ?
* Then should we forbid ```local``` target to generate remote/cross target except ```CrossCompiler``` ?
----
Would like to understand and see your reasoning on the subject first.
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772055866
> and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
@areusch
You are right. Also the public docs mentions the different context. Thanks a lot for your time !
Now, to avoid such confusions
- can we teach TVM to not invoke wrong procedure in wrong context (dropping error messages) ?
> Hi @areusch ,
>
> A follow up question, what are your thoughts on the effect of the usage of DefaultCompiler.library() function's support to get the operator library as an archive though? I can understand how this affects for DefaultCompiler.binary() or build_static_runtime (that uses DefaultCompiler.binary()). I guess my question is should we block DefaultCompiler.library() for cortex-m targets? -- or should we maybe factor that part out if we are blocking ?
The blocking shouldnt be done just by cortex-m (targets can be riscv, xtensa and many more). I think of blocking out all != ```micro(host)```
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771810267
and to clarify--i'd love to improve the documentation/tutorial, if there are specific parts you found confusing, i'd definitely be interested to learn that.
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772000660
Hi @areusch ,
A follow up question, what are your thoughts on the effect of the usage of DefaultCompiler.library() function's support to get the operator library as an archive though? I can understand how this affects for DefaultCompiler.binary() or build_static_runtime (that uses DefaultCompiler.binary())
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771789962
> hey @cbalint13 can you give an example code fragment that triggers this bug? i don't think we should be compiling rand_r to bare metal--the `main.cc` present in `src/runtime/crt/host/main.cc` is intended to be compiled against a traditional POSIX OS (e.g. is used for simulating a device as a subprocess and treating stdio as the UART link).
@areusch
Can look at this sample: [tvm-micro-pr7392.py](https://gist.github.com/cbalint13/68953f677c995a2b7933c1f0ffef474f#file-tvm-micro-pr7392-py)
----------------------------------------------------------------
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
[GitHub] [tvm] cbalint13 commented on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
cbalint13 commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772058444
> @areusch yes in the case where we just want to use export_library (w/o build_static_runtime) -- as of today we can provide DefaultCompiler.library() as the fcompile if one only wants get operator lib as an .a. I mean I understand there are other ways to do that such as modifying cc.cross_compile to support archive target with using a c compiler
Very good idea. But not sure that the ```micro``` backend should do this. But if you look at my gist example there already a save .o object file (and is not done by ```micro``` backend !).
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on pull request #7392: Fix rand_r() rpc proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771776674
----------------------------------------------------------------
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
[GitHub] [tvm] areusch edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
areusch edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-771968417
@cbalint13 it is a little bit more complicated than just specifying the Compiler implementation when cross-compiling. when using DefaultCompiler, there is assumption that all of the code needed to run the _platform_ (I.e. SoC startup code, peripheral config) lives in a `main.cc` or in a library you supply to `build_static_runtime(extra_libs=)`. on POSIX systems, this assumption doesn't impose much on the build, but in a bare metal deployment situation it's much different.
to increase compatibility with the many dev boards out there, we compile against a cross-platform deployment framework (Zephyr). most cross-platform bare-metal deployment frameworks include their own build system to determine which sources to include and the compiler config e.g. cflags, ldflags. Zephyr does this from CMakeLists.txt and prj.conf. making it convention to include this config in TVM Python scripts would mean that convention is to replicate the build system from the RTOS you're using.
unfortunately we kind of built out this Compiler abstraction without paying too much attention to that, so it's kind of the wrong level of abstraction to use here. the long-term solution to this problem is to replace Compiler with something more "project-level," where "project" roughly refers to the startup code, RTOS, and other application specific code you need to run models on a bare metal device. See #6 on the [µTVM M2 roadmap](https://discuss.tvm.apache.org/t/tvm-microtvm-m2-roadmap/8821).
to address your question B: you are still welcome to instantiate a Compiler impl of your choice and this doesn't need to live in the TVM source tree. until the project API refactor is done, happy to include additional impls into the TVM tree so long as they can be tested (see `tests/micro/qemu/test_zephyr.py`).
in the short term, I think it would be great to change DefaultCompiler to error when you try to compile a file with a cortex-m target. that would be adjusting the logic [here](https://github.com/apache/tvm/blob/main/python/tvm/micro/compiler.py#L101). want to take a shot at that?
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772026067
@areusch yes in the case where we just want to use export_library (w/o build_static_runtime) -- as of today we can provide DefaultCompiler.library() as the fcompile if one only wants get operator lib as an .a. I mean I understand there are other ways to do that such as modifying cc.cross_compile to support archive target with using a c compiler
----------------------------------------------------------------
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
[GitHub] [tvm] manupa-arm edited a comment on pull request #7392: Fix rand_r() proper inclusion
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #7392:
URL: https://github.com/apache/tvm/pull/7392#issuecomment-772000660
----------------------------------------------------------------
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