Skip to content

[onert] Use PkgConfig cmake module to find TRIXEngine#15273

Merged
hseok-oh merged 1 commit intoSamsung:masterfrom
batcheu:onert_fix_cmake
May 20, 2025
Merged

[onert] Use PkgConfig cmake module to find TRIXEngine#15273
hseok-oh merged 1 commit intoSamsung:masterfrom
batcheu:onert_fix_cmake

Conversation

@batcheu
Copy link
Copy Markdown
Contributor

@batcheu batcheu commented Apr 30, 2025

This commit replaces the use of the private cmake with the PkgConfig module. It will prevent a potential build failure caused by varying library paths.

ONE-DCO-1.0-Signed-off-by: Jonghwa Lee jonghwa3.lee@samsung.com

@batcheu batcheu requested a review from hseok-oh April 30, 2025 02:27
@batcheu batcheu marked this pull request as draft April 30, 2025 06:46
@batcheu batcheu marked this pull request as ready for review May 8, 2025 02:48
@batcheu
Copy link
Copy Markdown
Contributor Author

batcheu commented May 8, 2025

@hseok-oh
Ping ✨

Comment thread infra/nnfw/cmake/CfgOptionFlags.cmake Outdated
Comment on lines +8 to +9
find_package(PkgConfig REQUIRED)
pkg_check_modules(TRIXEngine REQUIRED IMPORTED_TARGET npu-engine)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change skips version check.
We use version check because our trix backend does not support version under 2.5.0.

set(TRYRUN_COMPILE_DEFINITIONS "-I${TRIXEngine_INCLUDE_DIR} -I${NPUBINFMT_INCLUDE_DIR}")
# TODO Assert TRIX_ENGINE_PREFIX is directory
# TODO Can we run this only once per configure?
try_run(MAJOR_VER MAJOR_COMPILABLE "${CMAKE_BINARY_DIR}/TRIXEngineConfigVersion.major"
SOURCES "${CMAKE_CURRENT_LIST_DIR}/TRIXEngine/TRIXEngineConfigVersion.major.cpp"
COMPILE_DEFINITIONS "${TRYRUN_COMPILE_DEFINITIONS}"
LINK_LIBRARIES ${TRIXEngine_LIB}
)
if(NOT MAJOR_COMPILABLE)
# This means VERSION < 2.2.7
# `getVersion` API introduced from TRIX Engine 2.2.7
if(PACKAGE_FIND_VERSION VERSION_GREATER_EQUAL 2.2.7)
message(STATUS "Fail to build TRIXEngine version checker")
set(PACKAGE_VERSION_EXACT FALSE)
set(PACKAGE_VERSION_COMPATIBLE FALSE)
set(PACKAGE_VERSION_UNSUITABLE TRUE)
return()
else()
# TODO How to support this case?
message(FATAL_ERROR "TRIX Engine version is too low (< 2.2.7)")
endif()
endif(NOT MAJOR_COMPILABLE)
try_run(MINOR_VER MINOR_COMPILABLE "${CMAKE_BINARY_DIR}/TRIXEngineConfigVersion.minor"
SOURCES "${CMAKE_CURRENT_LIST_DIR}/TRIXEngine/TRIXEngineConfigVersion.minor.cpp"
COMPILE_DEFINITIONS "${TRYRUN_COMPILE_DEFINITIONS}"
LINK_LIBRARIES ${TRIXEngine_LIB}
)
try_run(EXTRA_VER EXTRA_COMPILABLE "${CMAKE_BINARY_DIR}/TRIXEngineConfigVersion.extra"
SOURCES "${CMAKE_CURRENT_LIST_DIR}/TRIXEngine/TRIXEngineConfigVersion.extra.cpp"
COMPILE_DEFINITIONS "${TRYRUN_COMPILE_DEFINITIONS}"
LINK_LIBRARIES ${TRIXEngine_LIB}
)

So we need to get target using pkg_check_modules in infra/nnfw/cmake/packages/TRIXEngineConfig.cmake and infra/nnfw/cmake/packages/TRIXEngineConfigVersion.cmake indirectly with version check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added version requirement to `pkg_check_modules'.

+ # TRIXEngine version is required to higher than 2.5.0
+ pkg_check_modules(TRIXEngine REQUIRED IMPORTED_TARGET npu-engine>2.5.0)

@batcheu batcheu marked this pull request as draft May 19, 2025 07:53
@batcheu batcheu force-pushed the onert_fix_cmake branch from 10f15e2 to 0787ddc Compare May 19, 2025 07:55
nnfw_find_package(TRIXEngine QUIET 2.5.0)
find_package(PkgConfig REQUIRED)
# TRIXEngine version is required to higher than 2.5.0
pkg_check_modules(TRIXEngine REQUIRED IMPORTED_TARGET npu-engine>2.5.0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pkg_check_modules(TRIXEngine REQUIRED IMPORTED_TARGET npu-engine>2.5.0)
pkg_check_modules(TRIXEngine QUIET IMPORTED_TARGET npu-engine>2.5.0)

If it is REQUIRED, build fail when npu-engine dev package is not installed and below TRIXEngine_FOUND check becomes meaningless.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you're right, I'll fix it right away.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

This commit replaces the use of the private cmake with the PkgConfig module.
It will prevent a potential build failure caused by varying library paths.

ONE-DCO-1.0-Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Co-authored-by: Hyeongseok Oh <hseok82.oh@samsung.com>
@batcheu batcheu force-pushed the onert_fix_cmake branch from 0787ddc to e4b7e93 Compare May 20, 2025 05:37
@batcheu batcheu marked this pull request as ready for review May 20, 2025 05:38
Copy link
Copy Markdown
Contributor

@hseok-oh hseok-oh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@hseok-oh hseok-oh merged commit b1108f6 into Samsung:master May 20, 2025
11 checks passed
@batcheu batcheu deleted the onert_fix_cmake branch December 8, 2025 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants