Handle multiple opencl-c.h headers found during configure - #436
Draft
pvelesko wants to merge 2 commits into
Draft
Conversation
file(GLOB_RECURSE opencl-header ...) returns a list, and a single toolchain
installation can legitimately provide more than one opencl-c.h, for example
<prefix>/include/cclang/opencl-c.h installed by opencl-clang next to
<prefix>/lib/clang/<version>/include/opencl-c.h installed by clang itself.
Every consumer of opencl-header expects exactly one path, so the list silently
expands into extra arguments: get_filename_component() in
IGC/BiFModule/CMakeLists.txt fails outright, and get_bif_src_list() in
IGC/BiFModule/cmake/BiFModuleCache.cmake binds its arguments to the wrong
values and leaves the BiF checksum target with no dependencies at all.
The reproducer generates that toolchain layout and runs the real
igc_find_opencl_clang.cmake against it in two configurations:
- two headers and no override: opencl-header has to come out as exactly one
path, and the same one every time, and the two consumers above have to work
when it is dereferenced unquoted, which is how they dereference it today.
- two headers plus IGC_OPTION__OPENCL_HEADER_PATH: the documented override has
to win. It is ignored on the prebuilds branch today, which is the branch the
bug was reported on.
Each configuration is a child cmake configure, so a hard CMake error in one of
the replayed consumers is observed as a non-zero exit status instead of
aborting the run and hiding the remaining checks. It configures no compiler and
needs no LLVM, so it runs on its own in a couple of seconds:
cmake -S IGC/cmake/tests/opencl_header_multiple -B <some-build-dir>
This has to be run by hand. IGC has no CMake test harness at all: there is no
top-level enable_testing() and no add_test() anywhere in the tree, so there is
nothing to register the reproducer with.
Without the following commit the first configuration fails with
[multiple] number of opencl-c.h paths: expected '1', got '2'
and its child configure reproduces the traceback from the report,
get_filename_component unknown component <second header>
while the second fails with
[override] selected opencl-c.h: expected '<clang header>', got '<both>'
Signed-off-by: Paulius Velesko <pvelesko@pglc.io>
file(GLOB_RECURSE opencl-header ...) returns a list, and a single
toolchain installation can legitimately provide more than one
opencl-c.h, for example <prefix>/include/cclang/opencl-c.h installed by
opencl-clang next to <prefix>/lib/clang/<version>/include/opencl-c.h
installed by clang itself. Every consumer of opencl-header expects
exactly one path, so the list silently expands into extra arguments and
configure fails with
CMake Error at IGC/BiFModule/CMakeLists.txt:57 (get_filename_component):
get_filename_component unknown component <second header>
On the CCLANG_FROM_SYSTEM branch, where that get_filename_component()
call is skipped, the same list instead quietly mis-binds the arguments
of get_bif_src_list(), leaving the BiF checksum target with an empty
dependency list.
Add igc_select_opencl_header(), called after each of the two glob sites,
which reduces the candidates to the lexicographically first path so the
choice is deterministic and reports the ones it did not take. It also
honours IGC_OPTION__OPENCL_HEADER_PATH on the prebuilds branch, where
the problem was reported; previously the documented override only worked
when opencl-clang came from the system.
Fixes intel#314
Signed-off-by: Paulius Velesko <pvelesko@pglc.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
file(GLOB_RECURSE opencl-header ...)inIGC/cmake/igc_find_opencl_clang.cmakereturns a list whenever an installation ships more than oneopencl-c.h, for example<prefix>/include/cclang/opencl-c.hfrom opencl-clang next to<prefix>/lib/clang/<ver>/include/opencl-c.hfrom clang itself. Every consumer of${opencl-header}expects one path and dereferences it unquoted, so on the prebuilds branch configure dies withand on the
CCLANG_FROM_SYSTEMbranch the list instead silently shifts the arguments ofget_bif_src_list().IGC_OPTION__OPENCL_HEADER_PATHwas only consulted on theCCLANG_FROM_SYSTEMbranch, so it could not be used to work around the prebuilds case where this was reported.Add
igc_select_opencl_header()after both globs: honourIGC_OPTION__OPENCL_HEADER_PATHon every branch, and when the glob returned several candidates sort them, keep the first and print the choice.IGC has no CMake test harness, so the reproducer is a standalone project that needs no compiler, LLVM or network:
It builds a fake prebuilds tree with two
opencl-c.h, runs the module under test in a child configure per scenario and asserts on what it reported. On master it fails withnumber of opencl-c.h paths: expected '1', got '2'followed by the traceback above; with this change it exits 0, including the override scenario.#336 is a duplicate of this.
Fixes #314