-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[microTVM] additional refactoring for enabling USE_MICRO in more builds #13909
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
Conversation
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
| int TVMDeviceFreeDataSpace(DLDevice dev, void* ptr) { return TVMPlatformMemoryFree(ptr, dev); } | ||
|
|
||
| static bool IsContiguous(const DLTensor* arr) { | ||
| TVM_ATTRIBUTE_UNUSED static bool IsContiguous(const DLTensor* arr) { |
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.
Can you clarify why this was needed? was it for eliminating the warning?
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.
IsContiguous() is only used in an assert() below, so in a non-debug build we get an unused function warning
mehrdadh
left a comment
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, just waiting for others to take a look as well
| src/runtime/crt/microtvm_rpc_common/session.cc | ||
| src/runtime/crt/microtvm_rpc_common/write_stream.cc) | ||
| list(APPEND RUNTIME_SRCS ${TVM_CRT_SOURCES}) | ||
| include_directories(SYSTEM src/runtime/micro) |
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.
Why the SYSTEM 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.
it needs a header in that directory to build those files that are in TVM_CRT_SOURCES, it's the same pattern used in other .cmake and CmakeLists.txt
Additional changes that make it possible to enable USE_MICRO across nearly all builds except for hexagon or when USE_RPC is disabled. This mainly standardizes how the microTVM RPC common files are built across all platforms. There will be a follow-up PR that will enable USE_MICRO for nearly all build types once the best mechanism and behaviors for doing so is determined.