[Runtime] MISRA-C compliant TVM runtime#3934
Conversation
|
also cc @ajtulloch Now that we have quite a few runtimes, I wonder if it makes sense to consolidate some of them, for example, i do think it makes sense to make uTVM runtime MISRA-C compliant |
|
This PR also contains a rewritten version of JSON parser and NDArray reader, which is relative independent and concise. I would like to propose a replacement for the |
|
@liangfu absolutely, this would reduce code size substantially as well I believe. I suspect that it would make sense to remove the 'uTVM runtime' and replace it with the MISRA-C one - as long as you're comfortable with code size being an important consideration in the development of the MISRA-C one? |
|
@liangfu @ajtulloch would be great if we can followup on this topic. e.g. try to compare the standalone utvm and misra-c runtime, and review this part of the proposed code. One related question is that is it still OK to use C++? I see most of the code are implemented in pure C, which is fine, but perhaps we could still benefit from namespace and simple class(no virtual methods). |
|
The major intention to implement this in pure C is to maximize portability. (Some vendors didn't offer C++ compiler for its vision processor, partly for the sake of misra-c compliance, e.g. Vision SDK from TI.) On the other hand, this reduces binary size at the cost of flexibility. |
|
OK, I think we could go with c API, just remember to keep naming style consistent with the current C API as in c_runtime_api.h |
|
Sure, I would update the naming accordingly, and add test cases as well. |
|
ping @liangfu please see if you are interested in continue pushing this thread |
|
@tqchen sure, I can continue to push this thread, although this has been suspended for a while. |
|
@tqchen For now, the implement reproduces apps/bundle_deploy with complete runtime implemented in pure C. Note that the build_mode.py script generates apps/bundle_deploy_c/build/bridge.c to load functions in the compiled object (model.o), so that runtime could load all the compiled functions. In addition, to keep the pure C code OOP, an additional underscore is used in member functions for structs. Please take a review. |
|
This looks really neat, well done @liangfu. |
|
@ajtulloch Thanks for the comment. In addition, I was also trying to integrate this with your implement of the standalone uTVM runtime. Hopefully, we would have a successful removal of picojson as an external dependency. |
Change-Id: I027ddff15c31fb4da0bd0e461427dce619de1f93
I have tested this on both 32-bit ARM board, and 64-bit Linux PC, they produced exactly the same result.
Nice suggestion! I've made this runs in CI. Please take anther look. |
Change-Id: Ic2e82fb3051b6c254ef32a964f976b61e3e5fe4d
|
Excellent, thanks for adding this to CI so quickly! I was able to reproduce the demo by typing in |
Change-Id: Ide466a5cf21cf8bb990dcd4a1189ba17594e3c51
|
@liangfu The e2e code cannot be run as part of the unittest because the CPU module do not have mxnet installed. Let us create a simple function(e.g. elemwise add) that makes use of the runtime as the test function. |
|
ah, could we update the dockerfile to install mxnet? or will that be too wasteful in terms of CPU cycles |
|
In terms of testing the runtime out, using simple add one example would be sufficient. |
|
@liangfu would be great if you can try to update(use simple testing as in other all in one deployment that does not depend on mxnet) :) |
|
sure |
Change-Id: Ie0dfd0ade6be4665b4384db7d260a6c69b35010f
Change-Id: Ie7fbc16b4b0b9509918d986a841f443900813bef
|
@tmoreau89 @tqchen A simple test case has been added, and the testing results are successful. Please take another look. |
|
Thanks @liangfu @tmoreau89 @ajtulloch , this is now merged! |
|
Great work. @liangfu have you considered using "system lib" approach since dlopen is banned in some environments? |
|
@liangfu awesome work, very useful to our use-cases. |
* implement of MISRA-C compliant TVM runtime; * working on bundle_deploy_c demo * move header files into include dir * fix compatibility issues * fix compatibility issues * resolve most of the warnings and errros * implement c_backend_api * introduce bridge * working well * move to header files and bundle.c into src/runtime/crt * clean up * satisfy linter * clean up * test with the cat image * remove synset * refactoring * refactoring * refactoring * initial crt_runtime_api.c * improved compatibility with g++ * using exposed API in c_runtime_api.h * call from c_runtime_api.h * clean up * lint * merge into apps/bundle_deploy directory Change-Id: I51904db81b8589e65d107d8ca77b47452e3812b5 * make the demo runs in ci Change-Id: I2c24f8b592508833d3555311c2b24d1931f19385 * address review comments Change-Id: I027ddff15c31fb4da0bd0e461427dce619de1f93 * release Change-Id: I5ad5bb8426468aac9fc8d074e56ddea358a7fd91 * fix ci testing Change-Id: Ic2e82fb3051b6c254ef32a964f976b61e3e5fe4d * add test case for misra c runtime Change-Id: Ie0dfd0ade6be4665b4384db7d260a6c69b35010f * fread files in testing to avoid calling xxd Change-Id: Ie7fbc16b4b0b9509918d986a841f443900813bef
* implement of MISRA-C compliant TVM runtime; * working on bundle_deploy_c demo * move header files into include dir * fix compatibility issues * fix compatibility issues * resolve most of the warnings and errros * implement c_backend_api * introduce bridge * working well * move to header files and bundle.c into src/runtime/crt * clean up * satisfy linter * clean up * test with the cat image * remove synset * refactoring * refactoring * refactoring * initial crt_runtime_api.c * improved compatibility with g++ * using exposed API in c_runtime_api.h * call from c_runtime_api.h * clean up * lint * merge into apps/bundle_deploy directory Change-Id: I51904db81b8589e65d107d8ca77b47452e3812b5 * make the demo runs in ci Change-Id: I2c24f8b592508833d3555311c2b24d1931f19385 * address review comments Change-Id: I027ddff15c31fb4da0bd0e461427dce619de1f93 * release Change-Id: I5ad5bb8426468aac9fc8d074e56ddea358a7fd91 * fix ci testing Change-Id: Ic2e82fb3051b6c254ef32a964f976b61e3e5fe4d * add test case for misra c runtime Change-Id: Ie0dfd0ade6be4665b4384db7d260a6c69b35010f * fread files in testing to avoid calling xxd Change-Id: Ie7fbc16b4b0b9509918d986a841f443900813bef
|
May I ask some more details like which MISRA rules(MISRA-C 2004, MISRA-C++-2008, MISRA-C-2012) are fixed other than what is mentioned on the #3159 . Thanks! |
This PR implements MISRA-C compliant TVM runtime proposed in #3159 .
See timing improvements in the following table:
@tqchen @ajtulloch @nhynes Please review.