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 2022/10/19 16:00:12 UTC
[GitHub] [tvm] csullivan opened a new pull request, #13138: Support fetching and building Hexagon runtime with external runtime sources
csullivan opened a new pull request, #13138:
URL: https://github.com/apache/tvm/pull/13138
It can be desirable to incorporate external source libraries for Hexagon which are maintained outside of upstream TVM. This PR enables the cmake tvm option `USE_HEXAGON_EXTERNAL_LIBS` which can point to an external git repo containing runtime sources in `src/runtime/hexagon` for compilation along with the Hexagon TVM runtime.
When a git repo url is used, a SHA is required to be specified as well. Any source controlled use of TVM for benchmarking should commit the SHA used for reproducibility in the respective benchmarking repository -- and explicitly not in TVM.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999931498
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
+ elseif(USE_HEXAGON_EXTERNAL_LIBS MATCHES "\.git$")
+ if (NOT DEFINED HEXAGON_EXTERNAL_LIBS_SHA)
+ message(FATAL_ERROR "HEXAGON_EXTERNA_LIBS_SHA must be set when "
+ "USE_HEXAGON_EXTERNAL_LIBS is set to a git repository")
+ endif()
+ include(FetchContent)
+ FetchContent_Declare(hexagon_external
+ GIT_REPOSITORY "${USE_HEXAGON_EXTERNAL_LIBS}"
+ GIT_TAG "${HEXAGON_EXTERNAL_LIBS_SHA}")
+ FetchContent_MakeAvailable(hexagon_external)
+ set(USE_HEXAGON_EXTERNAL_LIBS "${hexagon_external_SOURCE_DIR}")
+ else()
+ message(FATAL_ERROR "Invalid use of USE_HEXAGON_EXTERNAL_LIBS="
+ "${USE_HEXAGON_EXTERNAL_LIBS}; USE_HEXAGON_EXTERNAL_LIBS only "
+ "supports absolute paths and paths and git repository urls")
+ endif()
+
+ file_glob_append(HEXAGON_EXTERNAL_RUNTIME_SRCS
+ "${USE_HEXAGON_EXTERNAL_LIBS}/src/runtime/hexagon/*.cc"
+ )
+ list(APPEND RUNTIME_HEXAGON_SRCS "${HEXAGON_EXTERNAL_RUNTIME_SRCS}")
+ set_source_files_properties(
+ "${HEXAGON_EXTERNAL_RUNTIME_SRCS}"
+ PROPERTIES COMPILE_FLAGS "-mhvx -mhmx"
Review Comment:
I was wondering the same also yesterday.
One option, that I personally prefer not to rely on, is to not allow external sources and require the external libs to fully manage their own build to object files which are linked in.
Another option that comes to mind would be to do a cmake include on a required hexagon_external.cmake in the external lib. I haven't explored the feasibility of this option but it could work.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999921614
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
Review Comment:
I should clear this up with a comment, thank you. EXISTS here is checking to see if USE_HEXAGON_EXTERNAL_LIBS is a path that is resolvable on the system. If it is a local path then it will use the path to the local install of external libs. EXISTS will return false when it's not a path, e.g. if it is a git url or malformed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] janetsc commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
janetsc commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999915616
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
Review Comment:
I'm a bit confused here. If we get here, USE_HEXAGON_EXTERNAL_LIBS exists, yes? When would line 188 be false?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] janetsc commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
janetsc commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999918384
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
+ elseif(USE_HEXAGON_EXTERNAL_LIBS MATCHES "\.git$")
+ if (NOT DEFINED HEXAGON_EXTERNAL_LIBS_SHA)
+ message(FATAL_ERROR "HEXAGON_EXTERNA_LIBS_SHA must be set when "
+ "USE_HEXAGON_EXTERNAL_LIBS is set to a git repository")
+ endif()
+ include(FetchContent)
+ FetchContent_Declare(hexagon_external
+ GIT_REPOSITORY "${USE_HEXAGON_EXTERNAL_LIBS}"
+ GIT_TAG "${HEXAGON_EXTERNAL_LIBS_SHA}")
+ FetchContent_MakeAvailable(hexagon_external)
+ set(USE_HEXAGON_EXTERNAL_LIBS "${hexagon_external_SOURCE_DIR}")
+ else()
+ message(FATAL_ERROR "Invalid use of USE_HEXAGON_EXTERNAL_LIBS="
+ "${USE_HEXAGON_EXTERNAL_LIBS}; USE_HEXAGON_EXTERNAL_LIBS only "
+ "supports absolute paths and paths and git repository urls")
+ endif()
+
+ file_glob_append(HEXAGON_EXTERNAL_RUNTIME_SRCS
+ "${USE_HEXAGON_EXTERNAL_LIBS}/src/runtime/hexagon/*.cc"
+ )
+ list(APPEND RUNTIME_HEXAGON_SRCS "${HEXAGON_EXTERNAL_RUNTIME_SRCS}")
+ set_source_files_properties(
+ "${HEXAGON_EXTERNAL_RUNTIME_SRCS}"
+ PROPERTIES COMPILE_FLAGS "-mhvx -mhmx"
Review Comment:
Could these properties specific to the external private repro be define there, and this pulls them in from a known sub-location within an external repo? (So that each private repo could customize flags inside itself.)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999985043
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
+ elseif(USE_HEXAGON_EXTERNAL_LIBS MATCHES "\.git$")
+ if (NOT DEFINED HEXAGON_EXTERNAL_LIBS_SHA)
+ message(FATAL_ERROR "HEXAGON_EXTERNA_LIBS_SHA must be set when "
+ "USE_HEXAGON_EXTERNAL_LIBS is set to a git repository")
+ endif()
+ include(FetchContent)
+ FetchContent_Declare(hexagon_external
+ GIT_REPOSITORY "${USE_HEXAGON_EXTERNAL_LIBS}"
+ GIT_TAG "${HEXAGON_EXTERNAL_LIBS_SHA}")
+ FetchContent_MakeAvailable(hexagon_external)
+ set(USE_HEXAGON_EXTERNAL_LIBS "${hexagon_external_SOURCE_DIR}")
+ else()
+ message(FATAL_ERROR "Invalid use of USE_HEXAGON_EXTERNAL_LIBS="
+ "${USE_HEXAGON_EXTERNAL_LIBS}; USE_HEXAGON_EXTERNAL_LIBS only "
+ "supports absolute paths and paths and git repository urls")
+ endif()
+
+ file_glob_append(HEXAGON_EXTERNAL_RUNTIME_SRCS
+ "${USE_HEXAGON_EXTERNAL_LIBS}/src/runtime/hexagon/*.cc"
+ )
+ list(APPEND RUNTIME_HEXAGON_SRCS "${HEXAGON_EXTERNAL_RUNTIME_SRCS}")
+ set_source_files_properties(
+ "${HEXAGON_EXTERNAL_RUNTIME_SRCS}"
+ PROPERTIES COMPILE_FLAGS "-mhvx -mhmx"
Review Comment:
Thank you for the good feedback, https://github.com/apache/tvm/pull/13138/commits/18fe01c722414045fbda3642c4f1a7ed608777f9 takes your suggestion of optionally including an external library defined cmake config file which defines these compiler flags. The risk we run is keeping these synchronized across repos, but as long as the footprint stays microscopic (ie just a single shared variable name) it should be manageable.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh merged pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #13138:
URL: https://github.com/apache/tvm/pull/13138
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tvm-bot commented on pull request #13138: Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13138:
URL: https://github.com/apache/tvm/pull/13138#issuecomment-1284242032
<!---bot-comment-->
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](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to auto-tag found, no teams are specified in PR title <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] janetsc commented on a diff in pull request #13138: [Hexagon] Support fetching and building Hexagon runtime with external runtime sources
Posted by GitBox <gi...@apache.org>.
janetsc commented on code in PR #13138:
URL: https://github.com/apache/tvm/pull/13138#discussion_r999915616
##########
cmake/modules/Hexagon.cmake:
##########
@@ -182,6 +182,36 @@ if(BUILD_FOR_HEXAGON)
"${TVMRT_SOURCE_DIR}/hexagon/ops/conv2d_fp16_hvx.cc"
PROPERTIES COMPILE_FLAGS "-mhvx"
)
+
+ # Include hexagon external library runtime sources
+ if(DEFINED USE_HEXAGON_EXTERNAL_LIBS)
+ if (EXISTS ${USE_HEXAGON_EXTERNAL_LIBS})
Review Comment:
I'm a bit confused here. Should line 188 check for HEXAGON_EXTERNAL_LIBS_SHA? If we get here, USE_HEXAGON_EXTERNAL_LIBS exists, yes?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org