[llvm-driver] Generate symlinks instead of executables for tools

[abrachet]

Summary

When LLVM_TOOL_LLVM_DRIVER_BUILD is On, create symlinks to llvm instead of creating the executables. Currently this only works for install and not install-distribution, the work for the later will be split up into a second patch.

[phosek]

clang/cmake/modules/AddClang.cmake

182

Should we check LLVM_TOOL_LLVM_DRIVER_BUILD here as well?

llvm/cmake/modules/AddLLVM.cmake

865–866

This name isn't particularly descriptive, how about setup_llvm_driver?

865–866

Most of these aren't being used in this function, could we prune this list and only keep the ones that are needed here?

914

Conversely, I think the arguments that are only used in this macro should be parsed here with cmake_parse_arguments.

2016

Should we check LLVM_TOOL_LLVM_DRIVER_BUILD here as well?

llvm/tools/CMakeLists.txt

67

Nit: This is superfluous.

llvm/tools/llvm-driver/CMakeLists.txt

34

The issue you described with subdirectory is a known limitation of CMake when using post build commands.

Do you need ALWAYS_GENERATE here? Have you tried it without that option. That would remove the limitation and I think it should be sufficient.

[mgorny]

Since you're working on this again, could you please kindly fix the regressions you've introduced?

https://reviews.llvm.org/D109977#3611683

We can't build clang for over a month now.

[abrachet]

Looks like I forgot to click send on these comments.

In D127800#3650787, @mgorny wrote:

Since you're working on this again, could you please kindly fix the regressions you've introduced?

https://reviews.llvm.org/D109977#3611683

We can't build clang for over a month now.

I've responded there

llvm/cmake/modules/AddLLVM.cmake

865–866

I don't think setup_llvm_driver makes sense because this is used mostly for add_llvm_executable.

865–866

Sure, that's a lot cleaner. The one hiccup I had was when this is called from add_clang_tool, SUPPORT_PLUGINS was getting propagated down, and there's no clean way (that I could think of) to eat that arg and not pass it to setup_llvm_executable. So I just parse it here even though it is unused. The alternative I could think of is to parse it in add_clang_tool.

llvm/tools/llvm-driver/CMakeLists.txt

34

Do you need ALWAYS_GENERATE here? Have you tried it without that option.

It's been a while so I don't totally remember every reason why I landed here. Though I remember this taking most of my time on the patch. I did start without it because the name made me think it likely wasn't what I wanted but for a couple of reasons I ended up using it.

The first big problem is there is both a library and clang symlink named clang-cpp, which without ALWAYS_GENERATE caused cmake to fail. It's not clear why that library is even called libclang-cpp it was changed in D64278 with no context as to why. It seemed they just changed it without too much fuss so likely it isn't a deal breaker if we really wanted to. But certainly annoying.

Also, if you do ninja llvm-cxxfilt it will build llvm but doesn't create the llvm-cxxfilt symlink. With ALWAYS_GENERATE it creates the symlink as I would expect.

FWIW, add_{clang,lld,flang}_symlink also all opt to use ALWAYS_GENERATE, though I suspect the latter two just copied what clang did.

That would remove the limitation and I think it should be sufficient.

I verified that you are right that I can get away without generate_driver_tool_targets in llvm/tools/CMakeLists.txt instead of hear if I omit the ALWAYS_GENERATE

[phosek]

LGTM

llvm/cmake/modules/AddLLVM.cmake

865–866

You could use https://cmake.org/cmake/help/v3.13/command/list.html#filter to filter it out of ${ARGN} in add_clang_tool before passing that to this macro but I'm not sure if that's cleaner.

865–866

The part I don't like is that it's not clear from the name what's the relationship between add_llvm_executable and setup_llvm_executable and when should I call which one (without knowing the details, I'd assume that I have to first add before I can setup). Maybe generate_llvm_objects would be better? This is very minor though.

llvm/tools/llvm-driver/CMakeLists.txt

34

Also, if you do ninja llvm-cxxfilt it will build llvm but doesn't create the llvm-cxxfilt symlink. With ALWAYS_GENERATE it creates the symlink as I would expect.

Thanks for the explanation, I think this alone is a reason to keep ALWAYS_GENERATE here.

[Amir]

llvm/cmake/modules/AddLLVM.cmake

1285

Sorry for a late question but I don't see any use of ARG_DEPENDS - is it intentional or there's an omission somewhere?

[abrachet]

llvm/cmake/modules/AddLLVM.cmake

1285

It looks like this was superfluous and likely copied from a place where DEPENDS is used. It's not necessary for depends to be used here, as it wasn't being used before for add_llvm_tool. I'll remove this

clang/cmake/modules/AddClang.cmake

 
 macro(add_clang_executable name)
   add_llvm_executable( ${name} ${ARGN} )
   set_target_properties(${name} PROPERTIES FOLDER "Clang executables")
   set_clang_windows_version_resource_properties(${name})
 endmacro(add_clang_executable)
 
 macro(add_clang_tool name)
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
   if (NOT CLANG_BUILD_TOOLS)
     set(EXCLUDE_FROM_ALL ON)
   endif()
-
+  if(ARG_GENERATE_DRIVER AND LLVM_TOOL_LLVM_DRIVER_BUILD)
+    setup_llvm_executable(${name} ${ARGN})
+    add_custom_target(${name} DEPENDS llvm-driver clang-resource-headers)
+  else()
     add_clang_executable(${name} ${ARGN})
     add_dependencies(${name} clang-resource-headers)
 
     if (CLANG_BUILD_TOOLS)
       get_target_export_arg(${name} Clang export_to_clangtargets)
       install(TARGETS ${name}
         ${export_to_clangtargets}
         RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
         COMPONENT ${name})
 
       if(NOT LLVM_ENABLE_IDE)
         add_llvm_install_targets(install-${name}
                                  DEPENDS ${name}
                                  COMPONENT ${name})
       endif()
       set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${name})
     endif()
+  endif()
 endmacro()
 
 macro(add_clang_symlink name dest)
+  get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
+  if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${dest} IN_LIST LLVM_DRIVER_TOOLS)
+    set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOL_SYMLINKS ${name})
+  else()
     add_llvm_tool_symlink(${name} ${dest} ALWAYS_GENERATE)
     # Always generate install targets
     llvm_install_symlink(${name} ${dest} ALWAYS_GENERATE)
+  endif()
 endmacro()
 
 function(clang_target_link_libraries target type)
+  if (TARGET obj.${target})
+    target_link_libraries(obj.${target} ${ARGN})
+  endif()
+
+  get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
+  if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${target} IN_LIST LLVM_DRIVER_TOOLS)
+    set(target llvm-driver)
+  endif()
+
   if (CLANG_LINK_CLANG_DYLIB)
     target_link_libraries(${target} ${type} clang-cpp)
   else()
     target_link_libraries(${target} ${type} ${ARGN})
   endif()
-  if (TARGET obj.${target})
-    target_link_libraries(obj.${target} ${ARGN})
-  endif()
-
 endfunction()

llvm/cmake/modules/AddLLVM.cmake

else()
     set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
   endif()
   if (ARG_MODULE)
     set_target_properties(${name} PROPERTIES FOLDER "Loadable modules")
   else()
     set_target_properties(${name} PROPERTIES FOLDER "Libraries")
   endif()
 endmacro(add_llvm_library name)
 
-macro(add_llvm_executable name)
-  cmake_parse_arguments(ARG
-    "DISABLE_LLVM_LINK_LLVM_DYLIB;IGNORE_EXTERNALIZE_DEBUGINFO;NO_INSTALL_RPATH;SUPPORT_PLUGINS;GENERATE_DRIVER"
-    "ENTITLEMENTS;BUNDLE_PATH"
-    "DEPENDS"
-    ${ARGN})
+macro(setup_llvm_executable name)
+  # SUPPORT_PLUGINS isn't actually used here, but we parse it so it
+  # doesn't show up as part of ARG_UNPARSED_ARGUMENTS.
+  cmake_parse_arguments(ARG "GENERATE_DRIVER;SUPPORT_PLUGINS" "" "DEPENDS" ${ARGN})
 
   llvm_process_sources( ALL_FILES ${ARG_UNPARSED_ARGUMENTS} )
 
   list(APPEND LLVM_COMMON_DEPENDS ${ARG_DEPENDS})
 
   # Generate objlib
   if(LLVM_ENABLE_OBJLIB OR ARG_GENERATE_DRIVER)
     # Generate an obj library for both targets.
if (ARG_GENERATE_DRIVER)
     set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_COMPONENTS ${LLVM_LINK_COMPONENTS})
     set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_DEPS ${ARG_DEPENDS} ${LLVM_COMMON_DEPENDS})
     set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_OBJLIBS "${obj_name}")
     
     set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOLS ${name})
     target_link_libraries(${obj_name} ${LLVM_PTHREAD_LIB})
     llvm_config(${obj_name} ${USE_SHARED} ${LLVM_LINK_COMPONENTS} )
   endif()
+endmacro()
 
+macro(add_llvm_executable name)
+  cmake_parse_arguments(ARG
+    "DISABLE_LLVM_LINK_LLVM_DYLIB;IGNORE_EXTERNALIZE_DEBUGINFO;NO_INSTALL_RPATH;SUPPORT_PLUGINS"
+    "ENTITLEMENTS;BUNDLE_PATH"
+    ""
+    ${ARGN})
+  setup_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
   add_windows_version_resource_file(ALL_FILES ${ALL_FILES})
 
   if(XCODE)
     # Note: the dummy.cpp source file provides no definitions. However,
     # it forces Xcode to properly link the static library.
     list(APPEND ALL_FILES "${LLVM_MAIN_SRC_DIR}/cmake/dummy.cpp")
   endif()
 
set (LLVM_TOOLCHAIN_TOOLS
     readelf
     size
     strings
     strip
     )
 endif()
 
 macro(add_llvm_tool name)
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
   if( NOT LLVM_BUILD_TOOLS )
     set(EXCLUDE_FROM_ALL ON)
   endif()
+  if(ARG_GENERATE_DRIVER AND LLVM_TOOL_LLVM_DRIVER_BUILD)
+    setup_llvm_executable(${name} ${ARGN})
+    add_custom_target(${name} DEPENDS llvm-driver)
+  else()
     add_llvm_executable(${name} ${ARGN})
 
     if ( ${name} IN_LIST LLVM_TOOLCHAIN_TOOLS OR NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
       if( LLVM_BUILD_TOOLS )
         get_target_export_arg(${name} LLVM export_to_llvmexports)
         install(TARGETS ${name}
                 ${export_to_llvmexports}
                 RUNTIME DESTINATION ${LLVM_TOOLS_INSTALL_DIR}
                 COMPONENT ${name})
 
         if (NOT LLVM_ENABLE_IDE)
           add_llvm_install_targets(install-${name}
                                   DEPENDS ${name}
                                   COMPONENT ${name})
         endif()
       endif()
     endif()
     if( LLVM_BUILD_TOOLS )
       set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
     endif()
     set_target_properties(${name} PROPERTIES FOLDER "Tools")
+  endif()
 endmacro(add_llvm_tool name)
 
 
 macro(add_llvm_example name)
   if( NOT LLVM_BUILD_EXAMPLES )
     set(EXCLUDE_FROM_ALL ON)
   endif()
   add_llvm_executable(${name} ${ARGN})
function(llvm_install_library_symlink name dest type)
 
   install(SCRIPT ${INSTALL_SYMLINK}
           CODE "install_symlink(${full_name} ${full_dest} ${output_dir})"
           COMPONENT ${component})
 
 endfunction()
 
 function(llvm_install_symlink name dest)
+  get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
+  if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${dest} IN_LIST LLVM_DRIVER_TOOLS)
+    set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOL_SYMLINKS ${name})
+    return()
+  endif()
   cmake_parse_arguments(ARG "ALWAYS_GENERATE" "COMPONENT" "" ${ARGN})
   foreach(path ${CMAKE_MODULE_PATH})
     if(EXISTS ${path}/LLVMInstallSymlink.cmake)
       set(INSTALL_SYMLINK ${path}/LLVMInstallSymlink.cmake)
       break()
     endif()
   endforeach()
 
   if(ARG_COMPONENT)
     set(component ${ARG_COMPONENT})
   else()
     if(ARG_ALWAYS_GENERATE)
       set(component ${dest})
     else()
       set(component ${name})
     endif()
   endif()
 
   set(full_name ${name}${CMAKE_EXECUTABLE_SUFFIX})
   set(full_dest ${dest}${CMAKE_EXECUTABLE_SUFFIX})
+  if (${dest} STREQUAL "llvm-driver")
+    set(full_dest llvm${CMAKE_EXECUTABLE_SUFFIX})
+  endif()
 
   install(SCRIPT ${INSTALL_SYMLINK}
           CODE "install_symlink(${full_name} ${full_dest} ${LLVM_TOOLS_INSTALL_DIR})"
           COMPONENT ${component})
 
   if (NOT LLVM_ENABLE_IDE AND NOT ARG_ALWAYS_GENERATE)
     add_llvm_install_targets(install-${name}
                              DEPENDS ${name} ${dest}

llvm/tools/CMakeLists.txt

 if(NOT LLVM_USE_INTEL_JITEVENTS )
   set(LLVM_TOOL_LLVM_JITLISTENER_BUILD Off)
 endif()
 
 if(CYGWIN OR NOT LLVM_ENABLE_PIC)
   set(LLVM_TOOL_LTO_BUILD Off)
 endif()
 
+if (LLVM_TOOL_LLVM_DRIVER_BUILD)
+  add_llvm_tool(llvm-driver)
+endif()
+
 # Add LTO, llvm-ar, llvm-config, and llvm-profdata before clang, ExternalProject
 # requires targets specified in DEPENDS to exist before the call to
 # ExternalProject_Add.
 add_llvm_tool_subdirectory(lto)
 add_llvm_tool_subdirectory(gold)
 add_llvm_tool_subdirectory(llvm-ar)
 add_llvm_tool_subdirectory(llvm-config)
 add_llvm_tool_subdirectory(llvm-lto)

 endforeach(p)
 
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} PARENT_SCOPE)
 
 if (LLVM_TOOL_LLVM_DRIVER_BUILD)
   # This is explicitly added at the end _after_ all tool projects so that it can
   # scrape up tools from other projects into itself.
   add_subdirectory(llvm-driver)
+  # This must be here otherwise CMake complains in add_llvm_tool_symlink that
+  # it can't add_custom_command that happens after llvm-driver is built because
+  # llvm-driver was not created in that directory.
+  generate_driver_tool_targets()
 endif()

llvm/tools/llvm-driver/CMakeLists.txt

 endforeach()
 
 get_property(LLVM_EXTRA_DRIVER_ENTRIES GLOBAL PROPERTY LLVM_EXTRA_DRIVER_ENTRIES)
 
 file(WRITE
           "${CMAKE_CURRENT_BINARY_DIR}/LLVMDriverTools.def"
           "${def_decl}${LLVM_EXTRA_DRIVER_ENTRIES}#undef LLVM_DRIVER_TOOL\n")
 
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-
-add_llvm_tool(llvm-driver
-  llvm-driver.cpp
-  )
+target_include_directories(llvm-driver PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
+target_sources(llvm-driver PRIVATE llvm-driver.cpp)
 
 set_target_properties(llvm-driver PROPERTIES OUTPUT_NAME llvm)
 
 target_link_libraries(llvm-driver PUBLIC ${LLVM_DRIVER_OBJLIBS})
 
 if(APPLE)
   # dsymutil uses some CoreFoundation stuff on Darwin...
   target_link_libraries(llvm-driver PRIVATE "-framework CoreFoundation")
 endif(APPLE)
+
+macro(generate_driver_tool_targets)
+  get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
+  get_property(LLVM_DRIVER_TOOL_SYMLINKS GLOBAL PROPERTY LLVM_DRIVER_TOOL_SYMLINKS)
+  foreach(name IN LISTS LLVM_DRIVER_TOOLS LLVM_DRIVER_TOOL_SYMLINKS)
+    add_llvm_tool_symlink(${name} llvm-driver ALWAYS_GENERATE)
+    # Always generate install targets
+    llvm_install_symlink(${name} llvm-driver ALWAYS_GENERATE)
+  endforeach()
+endmacro()