Skip to content

[SYCL][NFC] SYCL RT CMakeLists cleanup [5/N] #17419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

AlexeySachkov
Copy link
Contributor

Consolidated all common (i.e. non-conditional) linked libraries settings for SYCL RT build in a single place.

Consolidated all common (i.e. non-conditional) linked libraries settings
for SYCL RT build in a single place.
@AlexeySachkov AlexeySachkov marked this pull request as ready for review March 19, 2025 14:29
@AlexeySachkov AlexeySachkov requested a review from a team as a code owner March 19, 2025 14:29
Comment on lines -195 to -200
# Link UR
target_link_libraries(${LIB_NAME}
PRIVATE
UnifiedRuntime-Headers
UnifiedRuntimeCommon
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we don't want to link UnifiedRuntime-Headers, UnifiedRuntimeCommon libraries? I don't see linking these in the code above:

target_link_libraries(${LIB_NAME}
    PRIVATE
      ${CMAKE_DL_LIBS}
      ${CMAKE_THREAD_LIBS_INIT}
      $<$<NOT:$<BOOL:${WIN32}>>:UnifiedRuntimeLoader>
      $<$<BOOL:${WIN32}>:shlwapi>
  )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UR headers is an INTERFACE library - it only provides include directories and since LIB_NAME is a link-only target, we do not care about include directories there. Note that we still "link" that interface library to LIB_OBJ_NAME target, see #17413 and review comments there (similar question).

UR common seems to be an internal library which shouldn't be used by anyone outside of the UR. Once again, see the aforementioned #17413 and #17388 for confirmations from UR folks that we shouldn't use their common library

@uditagarwal97 uditagarwal97 merged commit d2bd8bc into intel:sycl Mar 20, 2025
22 checks passed
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