[hipDNN] Fix invalid exported include paths and remove old check cmake targets. (#3670)

## Motivation

Resolves ROCm/TheRock#2807

Also removes the unneeded cmake *check-old test targets as the new ctest
targets have proven sufficient.

## Technical Details

Issue was due to TheRock install propagating build paths for installed targets.
This fix removes the propagated build paths, and adds the proper include paths to the installed cmake config.

## Test Plan

Tests & build are passing correctly.
Adding a test to TheRock as a separate follow-up to emulate client consumption of hipDNN to prevent regressions.
Locally installing the artifacts, and using them to build samples from hipDNN.

## Test Result

Test & build are working.
Samples are now building and installing properly using artifacts generated with this change.


---------

Co-authored-by: BrianHarrisonAMD <169072757+BrianHarrisonAMD@users.noreply.github.com>

Author: CMiservaAMD

projects/hipdnn/CMakeLists.txt

@@ -189,8 +189,8 @@ if(HIP_DNN_BUILD_PLUGINS)
 endif()
 
 if(NOT HIP_DNN_SKIP_TESTS)
-    # keep this after all build folder have been added so they have a chance to register their tests
-    # using append_test_to_check_target
+    # Keep this after all build folders have been added so they have a chance to register their tests
+    # using add_*_test_target()
     finalize_test_targets()
 endif()
 

projects/hipdnn/backend/src/CMakeLists.txt

@@ -44,8 +44,6 @@ clang_tidy_check(hipdnn_backend_private)
 
 add_library(hipdnn_backend SHARED HipdnnBackend.cpp)
 
-target_include_directories(hipdnn_backend SYSTEM PUBLIC ${HIP_INCLUDE_DIRS})
-
 target_include_directories(
     hipdnn_backend
     PUBLIC $<BUILD_INTERFACE:${HIP_DNN_BACKEND_INCLUDE_DIR}>

projects/hipdnn/cmake/Dependencies.cmake

@@ -77,8 +77,10 @@ function(hipdnn_add_dependency_includes TARGET_NAME HEADER_LIB_TARGET_NAME)
     if(TARGET ${HEADER_LIB_TARGET_NAME})
         get_target_property(_dep_includes ${HEADER_LIB_TARGET_NAME} INTERFACE_INCLUDE_DIRECTORIES)
         if(_dep_includes)
-            message(VERBOSE "${TARGET_NAME} adding includes from ${HEADER_LIB_TARGET_NAME}: ${_dep_includes}")
-            target_include_directories(${TARGET_NAME} SYSTEM INTERFACE ${_dep_includes})
+            foreach(_include IN LISTS _dep_includes)
+                message(VERBOSE "${TARGET_NAME} adding include from ${HEADER_LIB_TARGET_NAME}: ${_include}")
+                target_include_directories(${TARGET_NAME} SYSTEM INTERFACE $<BUILD_INTERFACE:${_include}>)
+            endforeach()
         endif()
 
         if(ARG_COMPILE_DEFINITIONS)

projects/hipdnn/cmake/Tests.cmake

@@ -13,28 +13,8 @@ find_package(Python3 COMPONENTS Interpreter)
 
 findandcheckllvmsymbolizer()
 
-# Set executable prefix based on platform
-if(WIN32)
-    set(EXEC_PREFIX "")
-else()
-    set(EXEC_PREFIX "./")
-endif()
-
-set(CHECK_COMMAND_GLOBAL "" CACHE INTERNAL "Accumulated check commands" FORCE)
-set(CHECK_DEPENDS_GLOBAL "" CACHE INTERNAL "Accumulated check depends" FORCE)
-set(CHECK_EXECUTABLE_PATHS_GLOBAL "" CACHE INTERNAL "Accumulated check executable paths" FORCE)
-
-# Global collections for unit tests
-set(UNIT_CHECK_COMMAND_GLOBAL "" CACHE INTERNAL "Accumulated unit check commands" FORCE)
-set(UNIT_CHECK_DEPENDS_GLOBAL "" CACHE INTERNAL "Accumulated unit check depends" FORCE)
-
-# Global collections for integration tests
-set(INTEGRATION_CHECK_COMMAND_GLOBAL "" CACHE INTERNAL "Accumulated integration check commands"
-                                              FORCE
-)
-set(INTEGRATION_CHECK_DEPENDS_GLOBAL "" CACHE INTERNAL "Accumulated integration check depends"
-                                              FORCE
-)
+set(CHECK_DEPENDS_GLOBAL "" CACHE INTERNAL "Accumulated global dependencies for test name validation" FORCE)
+set(CHECK_EXECUTABLE_PATHS_GLOBAL "" CACHE INTERNAL "Accumulated global check executable paths" FORCE)
 
 # Builds the test environment list with optional code coverage support
 # ~~~
@@ -98,86 +78,6 @@ function(_create_test_name_validation_target_internal)
     endif() # Python3_FOUND
 endfunction() # _create_test_name_validation_target_internal
 
-# Generic internal function to append tests to check targets
-function(_append_test_to_check_target_internal TARGET WORKING_DIR TEST_TYPE STATUS_MESSAGE)
-    if(STATUS_MESSAGE)
-        message(STATUS "${STATUS_MESSAGE}: ${TARGET} in working directory: ${WORKING_DIR}")
-    endif()
-
-    if("${TEST_TYPE}" STREQUAL "UNIT")
-        set(COMMAND_VAR "UNIT_CHECK_COMMAND_GLOBAL")
-        set(DEPENDS_VAR "UNIT_CHECK_DEPENDS_GLOBAL")
-        set(CACHE_DESC "Accumulated unit check targets")
-    elseif("${TEST_TYPE}" STREQUAL "INTEGRATION")
-        set(COMMAND_VAR "INTEGRATION_CHECK_COMMAND_GLOBAL")
-        set(DEPENDS_VAR "INTEGRATION_CHECK_DEPENDS_GLOBAL")
-        set(CACHE_DESC "Accumulated integration check targets")
-    else()
-        set(COMMAND_VAR "CHECK_COMMAND_GLOBAL")
-        set(DEPENDS_VAR "CHECK_DEPENDS_GLOBAL")
-        set(CACHE_DESC "Accumulated check targets")
-    endif()
-
-    # Build environment list properly
-    _build_test_environment_list_internal(ENVIRONMENT_LIST)
-
-    set(NEW_COMMAND "")
-    if("${${COMMAND_VAR}}" STREQUAL "")
-        set(NEW_COMMAND ${CMAKE_COMMAND} -E env ${ENVIRONMENT_LIST}
-                        ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR}/${TARGET}
-        )
-    else()
-        set(NEW_COMMAND && ${CMAKE_COMMAND} -E env ${ENVIRONMENT_LIST}
-                        ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR}/${TARGET}
-        )
-    endif()
-
-    set(${COMMAND_VAR} ${${COMMAND_VAR}} ${NEW_COMMAND} CACHE INTERNAL "${CACHE_DESC}" FORCE)
-    set(${DEPENDS_VAR} ${${DEPENDS_VAR}} ${TARGET}
-        CACHE INTERNAL "Accumulated ${TEST_TYPE} check depends" FORCE
-    )
-
-    # Track the binary paths for test name validation
-    set(EXECUTABLE_PATH "${CMAKE_INSTALL_BINDIR}/${TARGET}")
-    set(CHECK_EXECUTABLE_PATHS_GLOBAL ${CHECK_EXECUTABLE_PATHS_GLOBAL} ${EXECUTABLE_PATH}
-        CACHE INTERNAL "Accumulated check executable paths" FORCE
-    )
-endfunction() # _append_test_to_check_target_internal
-
-# Generic internal function to finalize DIY check targets
-function(_finalize_check_target_internal TARGET_NAME COMMAND_VAR DEPENDS_VAR)
-    add_custom_target(
-        ${TARGET_NAME}
-        COMMAND ${${COMMAND_VAR}}
-        WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
-        DEPENDS ${${DEPENDS_VAR}}
-        VERBATIM
-        COMMENT "Running ${TARGET_NAME}"
-        USES_TERMINAL
-    )
-    message(VERBOSE "Created ${TARGET_NAME} target")
-endfunction() # _finalize_check_target_internal
-
-# Internal function to finalize the DIY unclassified check-old target
-function(_finalize_unclassified_check_target_internal)
-    _finalize_check_target_internal("check-old" "CHECK_COMMAND_GLOBAL" "CHECK_DEPENDS_GLOBAL")
-endfunction() # _finalize_unclassified_check_target_internal
-
-# Internal function to finalize the DIY unit-check-old target
-function(_finalize_unit_check_target_internal)
-    _finalize_check_target_internal(
-        "unit-check-old" "UNIT_CHECK_COMMAND_GLOBAL" "UNIT_CHECK_DEPENDS_GLOBAL"
-    )
-endfunction() # _finalize_unit_check_target_internal
-
-# Internal function to finalize the DIY integration-check-old target
-function(_finalize_integration_check_target_internal)
-    _finalize_check_target_internal(
-        "integration-check-old" "INTEGRATION_CHECK_COMMAND_GLOBAL"
-        "INTEGRATION_CHECK_DEPENDS_GLOBAL"
-    )
-endfunction() # _finalize_integration_check_target_internal
-
 enable_testing() # Cmake wont discover or run tests without this line
 
 # Internal helper function to create a ctest target
@@ -214,7 +114,7 @@ function(_add_ctest_target_internal TARGET_NAME LABEL VERBOSE COMMENT)
     message(VERBOSE "Created ${TARGET_NAME} target")
 endfunction() # _add_ctest_target_internal
 
-# Internal helper function to create the (new) check targets for running tests via ctest
+# Internal helper function to create the check targets for running tests via ctest
 function(_create_ctest_targets_internal)
     # cmake-format: off
     # Build test environment once for all ctest targets
@@ -238,12 +138,6 @@ function(finalize_test_targets)
 
     _create_ctest_targets_internal()
 
-    _finalize_unclassified_check_target_internal()
-    _finalize_unit_check_target_internal()
-    _finalize_integration_check_target_internal()
-
-    add_dependencies(check-old validate_test_names)
-
     # cmake-format: off
     # Create alias test targets without '_ctest' in the name
     # Regular targets (without --verbose)
@@ -259,36 +153,37 @@ function(finalize_test_targets)
     # cmake-format: on
 endfunction() # finalize_test_targets
 
-# Internal (old) DIY function to add a test target (assumes the test executable is a gtest
-# executable) Still needed for collecting list of test executables in HIPDNN_TEST_TARGETS and
-# setting test-type labels. TODO: Deprecate this funcion and remove manual test collection once new
-# ctest-based method is proven.
-function(_add_gtest_target_internal APPEND_FUNCTION_SUFFIX TARGET WORKING_DIR)
-    if("${APPEND_FUNCTION_SUFFIX}" STREQUAL "test")
-        _append_test_to_check_target_internal(
-            ${TARGET} ${WORKING_DIR} "" "Appending unclassified check target"
-        )
-    elseif("${APPEND_FUNCTION_SUFFIX}" STREQUAL "unit_test")
-        _append_test_to_check_target_internal(
-            ${TARGET} ${WORKING_DIR} "UNIT" "Appending unit check target"
-        )
-        _append_test_to_check_target_internal(${TARGET} ${WORKING_DIR} "" "")
-    elseif("${APPEND_FUNCTION_SUFFIX}" STREQUAL "integration_test")
-        _append_test_to_check_target_internal(
-            ${TARGET} ${WORKING_DIR} "INTEGRATION" "Appending integration check target"
-        )
-        _append_test_to_check_target_internal(${TARGET} ${WORKING_DIR} "" "")
-    else()
-        message(FATAL_ERROR "Unknown test type suffix: ${APPEND_FUNCTION_SUFFIX}")
-    endif()
+# ~~~
+# Internal helper function to record, configure, and register a ctest test target. Assumes that the
+# test target is a gtest executable, setting up:
+# - Test name validation tracking (adds to global dependency and executable path lists)
+# - RPATH settings for relocatable test executables
+# - Installation rules for test binaries
+# - CTest registration with appropriate labels (e.g. unit / integration test labels)
+# Parameters:
+#   APPEND_FUNCTION_SUFFIX - Label to apply to the test (e.g., "unit_test", "integration_test", "test")
+#   TARGET - Name of the test executable target (must already exist)
+#   WORKING_DIR - Working directory for test execution
+# ~~~
+function(_add_test_target_internal APPEND_FUNCTION_SUFFIX TARGET WORKING_DIR)
+    message(STATUS "Appending ${APPEND_FUNCTION_SUFFIX} check target: ${TARGET} in working directory: ${WORKING_DIR}")
 
-    set_target_properties(
-        ${TARGET} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR}"
+    # Track the dependencies for test name validation
+    set(CHECK_DEPENDS_GLOBAL ${CHECK_DEPENDS_GLOBAL} ${TARGET}
+        CACHE INTERNAL "Accumulated global dependencies for test name validation" FORCE
+    )
+    # Track the binary paths for test name validation
+    set(CHECK_EXECUTABLE_PATHS_GLOBAL ${CHECK_EXECUTABLE_PATHS_GLOBAL} "${CMAKE_INSTALL_BINDIR}/${TARGET}"
+        CACHE INTERNAL "Accumulated global check executable paths" FORCE
     )
 
     # Track this test target for later use in generating installed CTestTestfile.cmake
     set_property(GLOBAL APPEND PROPERTY HIPDNN_TEST_TARGETS ${TARGET})
 
+    set_target_properties(
+        ${TARGET} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR}"
+    )
+
     # Make test executables relocatable so they can find libraries when build directory is moved
     # Include both the main lib directory and the engine plugin directories
     set_target_properties(
@@ -309,17 +204,17 @@ endfunction() # _add_gtest_target_internal
 
 # Adds a generic test target
 function(add_unclassified_test_target TARGET WORKING_DIR)
-    _add_gtest_target_internal(test ${TARGET} ${WORKING_DIR})
+    _add_test_target_internal(test ${TARGET} ${WORKING_DIR})
 endfunction() # add_unclassified_test_target
 
 # Adds a unit test target
 function(add_unit_test_target TARGET WORKING_DIR)
-    _add_gtest_target_internal(unit_test ${TARGET} ${WORKING_DIR})
+    _add_test_target_internal(unit_test ${TARGET} ${WORKING_DIR})
 endfunction() # add_unit_test_target
 
 # Adds an integration test target
 function(add_integration_test_target TARGET WORKING_DIR)
-    _add_gtest_target_internal(integration_test ${TARGET} ${WORKING_DIR})
+    _add_test_target_internal(integration_test ${TARGET} ${WORKING_DIR})
 endfunction() # add_integration_test_target
 
 # Install CTest configuration files for direct test execution This should be called once at the end

projects/hipdnn/data_sdk/cmake/hipdnn_data_sdkConfig_imported.cmake.in

@@ -3,6 +3,41 @@
 
 @PACKAGE_INIT@
 
+# Extract and use include directories from dependency targets instead of linking
+# Function to add include directories and optional compile definitions from dependency targets
+# !!!! Note !!!! We are using this to force a headeronly consumption of 3rd party dependencies for hipdnn_data_sdk
+# Using regular linking will result in a non-header only consumption of these libraries due to the way they are configured when installing
+function(hipdnn_add_dependency_includes TARGET_NAME HEADER_LIB_TARGET_NAME)
+    # Parse optional arguments
+    set(options "")
+    set(oneValueArgs "")
+    set(multiValueArgs COMPILE_DEFINITIONS)
+    cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+    # Validate required parameters
+    if(NOT TARGET ${TARGET_NAME})
+        message(FATAL_ERROR "hipdnn_add_dependency_includes: Target '${TARGET_NAME}' does not exist")
+        return()
+    endif()
+
+    if(NOT TARGET ${HEADER_LIB_TARGET_NAME})
+        message(FATAL_ERROR "hipdnn_add_dependency_includes: Header library target '${HEADER_LIB_TARGET_NAME}' does not exist")
+        return()
+    endif()
+
+    if(TARGET ${HEADER_LIB_TARGET_NAME})
+        get_target_property(_dep_includes ${HEADER_LIB_TARGET_NAME} INTERFACE_INCLUDE_DIRECTORIES)
+        if(_dep_includes)
+            message(VERBOSE "${TARGET_NAME} adding includes from ${HEADER_LIB_TARGET_NAME}: ${_dep_includes}")
+            target_include_directories(${TARGET_NAME} SYSTEM INTERFACE ${_dep_includes})
+        endif()
+
+        if(ARG_COMPILE_DEFINITIONS)
+            target_compile_definitions(${TARGET_NAME} INTERFACE ${ARG_COMPILE_DEFINITIONS})
+        endif()
+    endif()
+endfunction()
+
 # Find required dependencies
 find_package(Threads REQUIRED)
 
@@ -12,6 +47,9 @@ find_dependency(flatbuffers REQUIRED)
 find_dependency(spdlog REQUIRED)
 find_dependency(nlohmann_json REQUIRED)
 
+# fmt can potentially be embedded in spdlog, make this dependency quiet, and conditional include the path
+find_dependency(fmt QUIET)
+
 # Subdirectory path for hipDNN engine plugins (relative to lib)
 set(HIPDNN_PLUGIN_ENGINE_SUBDIR "@HIPDNN_PLUGIN_ENGINE_SUBDIR@")
 
@@ -21,6 +59,16 @@ set(HIPDNN_RELATIVE_INSTALL_PLUGIN_ENGINE_DIR "@HIPDNN_RELATIVE_INSTALL_PLUGIN_E
 
 include("${CMAKE_CURRENT_LIST_DIR}/hipdnn_data_sdkTargets.cmake")
 
+hipdnn_add_dependency_includes(hipdnn_data_sdk flatbuffers::flatbuffers)
+hipdnn_add_dependency_includes(hipdnn_data_sdk spdlog::spdlog_header_only)
+hipdnn_add_dependency_includes(hipdnn_data_sdk nlohmann_json::nlohmann_json)
+if (fmt_FOUND)
+    message(STATUS "Found fmt: using system fmt")
+    hipdnn_add_dependency_includes(hipdnn_data_sdk fmt::fmt COMPILE_DEFINITIONS SPDLOG_FMT_EXTERNAL)
+else()
+    message(STATUS "Not found fmt: using bundled fmt from spdlog")
+endif()
+
 message(STATUS "hipDNN Data SDK: Engine plugin build directory is ${CMAKE_INSTALL_LIBDIR}/${HIPDNN_PLUGIN_ENGINE_SUBDIR}")
 message(STATUS "hipDNN Data SDK: Plugin absolute installation directory ${HIPDNN_FULL_INSTALL_PLUGIN_ENGINE_DIR}")
 message(STATUS "hipDNN Data SDK: Plugin relative installation directory ${HIPDNN_RELATIVE_INSTALL_PLUGIN_ENGINE_DIR}")

projects/hipdnn/plugins/miopen_legacy_plugin/CMakeLists.txt

@@ -159,14 +159,9 @@ if(NOT HIP_DNN_SKIP_TESTS)
     add_subdirectory(integration_tests)
 
     if(NOT BUILD_PLUGIN_AS_DEPENDENCY)
-        create_test_name_validation_target()
-
-        finalize_custom_check_target()
-        finalize_unit_check_target()
-        finalize_integration_check_target()
-
-        add_dependencies(check validate_test_names)
-        add_dependencies(check_ctest validate_test_names)
+        # Keep this after all build folders have been added so they have a chance to register their tests
+        # using add_*_test_target()
+        finalize_test_targets()
 
         # For standalone builds we need to install the ctest files
         install_miopen_legacy_plugin_ctest_files()