Lunderberg commented on a change in pull request #8822:
URL: https://github.com/apache/tvm/pull/8822#discussion_r694916346
##########
File path: cmake/modules/Hexagon.cmake
##########
@@ -47,41 +47,10 @@ function(find_hexagon_toolchain)
endif()
endfunction()
-function(find_hexagon_sdk_root)
- if(FOUND_HEXAGON_SDK_ROOT)
- return()
- endif()
- message(STATUS "Checking Hexagon SDK root: ${USE_HEXAGON_SDK}")
- file(GLOB_RECURSE HEXAGON_AEESTDDEF "${USE_HEXAGON_SDK}/*/AEEStdDef.h")
- if(HEXAGON_AEESTDDEF)
- # The path is ${HEXAGON_SDK_ROOT}/incs/stddef/AEEStdDef.h.
- get_filename_component(HEXAGON_TMP0 "${HEXAGON_AEESTDDEF}" DIRECTORY)
- get_filename_component(HEXAGON_TMP1 "${HEXAGON_TMP0}" DIRECTORY)
- get_filename_component(HEXAGON_TMP2 "${HEXAGON_TMP1}" DIRECTORY)
- set(HEXAGON_SDK_ROOT "${HEXAGON_TMP2}" CACHE PATH
- "Root directory of Hexagon SDK")
- set(FOUND_HEXAGON_SDK_ROOT TRUE)
- else(HEXAGON_AEESTDDEF)
- message(SEND_ERROR "Cannot validate Hexagon SDK in ${USE_HEXAGON_SDK}")
- endif()
-endfunction()
-
if(BUILD_FOR_HEXAGON)
- find_hexagon_sdk_root()
- if(HEXAGON_SDK_ROOT MATCHES "3.5.1")
- message(SEND_ERROR "Hexagon SDK 3.5.1 is not supported")
- elseif(HEXAGON_SDK_ROOT MATCHES "3\.[0-9]+\.[0-9]+")
- include_directories(
- SYSTEM "${USE_HEXAGON_SDK}/libs/common/qurt/ADSPv62MP/include/posix"
- SYSTEM "${USE_HEXAGON_SDK}/libs/common/qurt/ADSPv62MP/include/qurt")
- else()
- include_directories(
- SYSTEM "${HEXAGON_SDK_ROOT}/rtos/qurt/computev65/include/posix"
- SYSTEM "${HEXAGON_SDK_ROOT}/rtos/qurt/computev65/include/qurt")
- endif()
- include_directories(
- SYSTEM "${HEXAGON_SDK_ROOT}/incs"
- SYSTEM "${HEXAGON_SDK_ROOT}/incs/stddef")
+ find_hexagon_sdk_root("${USE_HEXAGON_SDK}" "v66")
+ # Add SDK and QuRT includes when building for Hexagon.
+ include_directories(SYSTEM ${HEXAGON_SDK_INCLUDES} ${HEXAGON_QURT_INCLUDES})
Review comment:
I think that would also require moving the
[`include(cmake/modules/Hexagon.cmake)`](https://github.com/Lunderberg/tvm/blob/main/CMakeLists.txt#L358)
to be after the definitions of `tvm_objs`. It would then also require using
the `target_sources` function instead of appending to the `RUNTIME_SRCS`
variable, since the `RUNTIME_SRCS` variable isn't used after the definition of
the object libraries.
If we're cleaning up the module usage, I'd lean toward having every module
define an object library, along with any flags that are needed. It looks like
there's a [nice new feature in cmake
3.12](https://cmake.org/cmake/help/latest/command/add_library.html#object-libraries)
that allows object libraries to specify linkage. There are some workarounds
for earlier versions, including using static libraries, but needing the
`-Wl,--whole-archive` flag is causing some issues.
I think I'd stick with the current style for consistency with other modules,
but I want to change it to use object libraries if/when we require a newer
version of cmake.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
