[PATCH] D117061: [BOLT][CMAKE] Accept BOLT_CLANG_EXE and BOLT_LLD_EXE

Amir Ayupov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 13 21:40:36 PST 2022


Amir added inline comments.


================
Comment at: bolt/CMakeLists.txt:19
 if (LLVM_INCLUDE_TESTS)
   string(FIND "${LLVM_ENABLE_PROJECTS}" "clang" POSITION)
+  if (NOT ${POSITION} EQUAL -1 OR BOLT_CLANG_EXE)
----------------
smeenai wrote:
> Not your diff, but CMake has `if(IN_LIST)` now, which you can use instead of the `string(FIND)`, e.g.
> ```
> if("clang" IN_LIST LLVM_ENABLE_PROJECTS)
> ```
Thanks! Will fix in a separate diff.


================
Comment at: bolt/test/CMakeLists.txt:17
+if (BOLT_CLANG_EXE)
+  add_executable(clang IMPORTED GLOBAL)
+  set_property(TARGET clang PROPERTY IMPORTED_LOCATION "${BOLT_CLANG_EXE}")
----------------
smeenai wrote:
> Hmm, will these targets not clash with the `clang` and `lld` targets from those LLVM subprojects, if you have those enabled? Is the assumption that you would only use `BOLT_CLANG_EXE` and `BOLT_LLD_EXE` if you're not enabling those subprojects?
Yes, that's the assumption. I thought about it a bit more and realized that if clang project is enabled, it will be picked up by logic in lit.cfg.py because BOLT_CLANG_EXE directory is used as an extra search path.

So the fix should be as follows:
1. Emit a warning in bolt/CMakeLists.txt if BOLT_CLANG_EXE is set and clang project is enabled about which one will be used (built-in clang).
2. Only add the clang target if BOLT_CLANG_EXE is set but clang project is not enabled to satisfy the dependency on clang in BOLT_TEST_DEPS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117061/new/

https://reviews.llvm.org/D117061



More information about the llvm-commits mailing list