Skip to content

Conversation

wenju-he
Copy link
Contributor

Commit df74736 placed libclc libraries into clang resource dir /lib/libclc at build stage.
This PR does it at install stage as well.
Note that in standalone (not in-tree) build, libclc is still installed to old ${CMAKE_INSTALL_DATADIR}/clc dir.

…c in in-tree build

Commit df74736 placed libclc libraries into clang resource dir
<resource-dir>/lib/libclc at build stage.
This PR does it at install stage as well.
Note that in standalone (not in-tree) build, libclc is still installed
to old ${CMAKE_INSTALL_DATADIR}/clc dir.
@wenju-he wenju-he requested a review from Copilot October 17, 2025 00:10
@llvmbot llvmbot added the libclc libclc OpenCL library label Oct 17, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adjust installation path for libclc so that in an in-tree build the libraries install into Clang's resource directory (lib/libclc), matching prior build-stage behavior; standalone build keeps the old datadir path.

  • Introduces LIBCLC_INSTALL_DIR variable for both standalone and in-tree configurations.
  • Reworks path construction for in-tree build using get_clang_resource_dir and cmake_path commands.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libclc/cmake/modules/AddLibclc.cmake Switch install destination to use computed LIBCLC_INSTALL_DIR variable.
libclc/CMakeLists.txt Define LIBCLC_INSTALL_DIR and alter in-tree output and install path logic using get_clang_resource_dir and cmake_path.

set( LIBCLC_OUTPUT_LIBRARY_DIR ${LIBCLC_OUTPUT_DIR}/lib/libclc )
cmake_path( APPEND LIBCLC_INSTALL_DIR "lib" "libclc" )
cmake_path( GET LLVM_LIBRARY_OUTPUT_INTDIR PARENT_PATH LIBCLC_OUTPUT_LIBRARY_DIR )
cmake_path( APPEND LIBCLC_OUTPUT_LIBRARY_DIR ${LIBCLC_INSTALL_DIR} )
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

Appending an (likely) absolute path ${LIBCLC_INSTALL_DIR} (after line 107) to LIBCLC_OUTPUT_LIBRARY_DIR (line 109) can discard the earlier parent path per cmake_path APPEND semantics when an absolute component is used, resulting in LIBCLC_OUTPUT_LIBRARY_DIR incorrectly mirroring the install directory. If the intent is to place build artifacts under the build tree while installing to the resource dir, compute a separate resource path (e.g., get_clang_resource_dir with PREFIX for build output) or append only relative components (lib, libclc) instead of the full absolute path.

Suggested change
cmake_path( APPEND LIBCLC_OUTPUT_LIBRARY_DIR ${LIBCLC_INSTALL_DIR} )
cmake_path( APPEND LIBCLC_OUTPUT_LIBRARY_DIR "lib" "libclc" )

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Appending an (likely) absolute path ${LIBCLC_INSTALL_DIR}

not true, get_clang_resource_dir always returns a relative path

@wenju-he wenju-he merged commit b9e2f7a into llvm:main Oct 20, 2025
10 checks passed
@wenju-he wenju-he deleted the libclc-install-dir branch October 20, 2025 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libclc libclc OpenCL library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants