-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[runtimes] Allow use of external llvm-lit on standalone builds #144347
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
Conversation
When creating a standalone build of the runtimes sub-project, the current CMake implementation looks for a lit executable that might potentially exist in the build tree and unconditionally overrides the value of `LLVM_EXTERNAL_LIT`. Due to this, any value passed via `-DLLVM_EXTERNAL_LIT` when configuring the CMake project is ignored. This change adds the `ALLOW_EXTERNAL` argument to the `get_llvm_lit_path` call in the runtimes' CMakeLists.txt, allowing any value previously set to be considered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine to me, although the setting of LLVM_EXTERNAL_LIT_MISSING_WARNED_ONCE
is incorrect if LLVM_EXTERNAL_LIT
was already set when entering this file. Probably doesn't matter since people who manually set LLVM_EXTERNAL_LIT
probably know what they are doing and if it is set to something invalid, then we just fail a bit later in the configure process instead.
Thanks for the review! When llvm-project/llvm/cmake/modules/AddLLVM.cmake Lines 2039 to 2050 in 64bd485
|
Ah yes, sounds good to me then :) |
…144347) When creating a standalone build of the runtimes sub-project, the current CMake implementation looks for a lit executable that might potentially exist in the build tree and unconditionally overrides the value of `LLVM_EXTERNAL_LIT`. Due to this, any value passed via `-DLLVM_EXTERNAL_LIT` when configuring the CMake project is ignored. This change adds the `ALLOW_EXTERNAL` argument to the `get_llvm_lit_path` call in the runtimes' CMakeLists.txt, allowing any value previously set to be considered.
When creating a standalone build of the runtimes sub-project, the
current CMake implementation looks for a lit executable that might
potentially exist in the build tree and unconditionally overrides the
value of
LLVM_EXTERNAL_LIT
. Due to this, any value passed via-DLLVM_EXTERNAL_LIT
when configuring the CMake project is ignored.This change adds the
ALLOW_EXTERNAL
argument to theget_llvm_lit_path
call in the runtimes' CMakeLists.txt, allowing anyvalue previously set to be considered.