[GitHub] [tvm] Lunderberg commented on a change in pull request #8822: [Hexagon] Reuse Hexagon SDK analysis across cmake files

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org