[PATCH] D131310: [llvm-driver] Support single distributions

Petr Hosek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 28 10:36:00 PDT 2022


phosek added a comment.

Since distributions are used for selecting components that are distributed in the toolchain, and most vendors only have a single distribution, I'd prefer using the name `llvm` (rather than `llvm-distribution`) for the default distribution since that binary is going to be shipped in the toolchain (and "distribution" is an internal  concept that users shouldn't be concerned with).

I wonder if we could use the following approach:

- If `LLVM_DISTRIBUTION_COMPONENTS` isn't set, then combine all supported tools into `llvm` driver.
- If `LLVM_DISTRIBUTION_COMPONENTS` is set, then combine all supported tools listed in `LLVM_DISTRIBUTION_COMPONENTS` into `llvm` driver and leave all other tools standalone.
- (In the future) If `LLVM_DISTRIBUTIONS` is set, then combine all supported tools listed in `LLVM_<name>_DISTRIBUTION_COMPONENTS` into `llvm-<name>` driver and leave all other tools standalone.

What do you think?



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:2049
+  if (${dest} MATCHES "llvm-driver.*")
+  string(REPLACE "-driver" "" normalized_dest ${dest})
+    set(full_dest ${normalized_dest}${CMAKE_EXECUTABLE_SUFFIX})
----------------
Should this apply to `${dest}` as well?


================
Comment at: llvm/tools/llvm-driver/CMakeLists.txt:31
 
-target_include_directories(llvm-driver PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
-target_sources(llvm-driver PRIVATE llvm-driver.cpp)
+  set(deps "")
+  foreach(objlib ${LLVM_DRIVER_OBJLIBS})
----------------
Nit: you can omit, variables in CMake don't need to be initialized.


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

https://reviews.llvm.org/D131310



More information about the llvm-commits mailing list