Skip to content
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

[llvm] Revise IDE folder structure #89741

Merged
merged 5 commits into from
May 25, 2024
Merged

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Apr 23, 2024

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the LLVM part that the other patches are going to depend. See #89153 for motivation.

Update the folder titles for targets in the monorepository that have not seen taken care of for some time. These are the folders that targets are organized in Visual Studio and XCode (set_property(TARGET <target> PROPERTY FOLDER "<title>")) when using the respective CMake's IDE generator.

  • Ensure that every target is in a folder
  • Use a folder hierarchy with each LLVM subproject as a top-level folder
  • Use consistent folder names between subprojects
  • When using target-creating functions from AddLLVM.cmake, automatically deduce the folder. This reduces the number of set_property/set_target_property, but are still necessary when add_custom_target, add_executable, add_library, etc. are used. A LLVM_SUBPROJECT_TITLE definition is used for that in each subproject's root CMakeLists.txt.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 23, 2024

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-mlgo
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-tools-llvm-exegesis
@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-backend-webassembly

Author: Michael Kruse (Meinersbur)

Changes

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the LLVM part that the other patches are going to depend. See #89153 for motivation.

Update the folder titles for targets in the monorepository that have not seen taken care of for some time. These are the folders that targets are organized in Visual Studio and XCode (set_property(TARGET &lt;target&gt; PROPERTY FOLDER "&lt;title&gt;")) when using the respective CMake's IDE generator.

  • Ensure that every target is in a folder
  • Use a folder hierarchy with each LLVM subproject as a top-level folder
  • Use consistent folder names between subprojects
  • When using target-creating functions from AddLLVM.cmake, automatically deduce the folder. This reduces the number of set_property/set_target_property, but are still necessary when add_custom_target, add_executable, add_library, etc. are used. A LLVM_SUBPROJECT_TITLE definition is used for that in each subproject's root CMakeLists.txt.

Patch is 52.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89741.diff

56 Files Affected:

  • (modified) llvm/CMakeLists.txt (+8-4)
  • (modified) llvm/cmake/modules/AddLLVM.cmake (+62-21)
  • (modified) llvm/cmake/modules/AddOCaml.cmake (+3-2)
  • (modified) llvm/cmake/modules/AddSphinxTarget.cmake (+3)
  • (modified) llvm/cmake/modules/CrossCompile.cmake (+4)
  • (modified) llvm/cmake/modules/LLVMDistributionSupport.cmake (+8)
  • (modified) llvm/cmake/modules/LLVMExternalProjectUtils.cmake (+24-1)
  • (modified) llvm/cmake/modules/TableGen.cmake (+6-1)
  • (modified) llvm/docs/CMakeLists.txt (+1)
  • (modified) llvm/examples/Kaleidoscope/CMakeLists.txt (+1-1)
  • (modified) llvm/include/llvm/Support/CMakeLists.txt (+1-1)
  • (modified) llvm/lib/Support/BLAKE3/CMakeLists.txt (+1)
  • (modified) llvm/runtimes/CMakeLists.txt (+23)
  • (modified) llvm/test/CMakeLists.txt (+3-3)
  • (modified) llvm/tools/opt-viewer/CMakeLists.txt (+1)
  • (modified) llvm/unittests/Analysis/InlineAdvisorPlugin/CMakeLists.txt (+1-2)
  • (modified) llvm/unittests/Analysis/InlineOrderPlugin/CMakeLists.txt (+1-1)
  • (modified) llvm/unittests/CMakeLists.txt (+1-1)
  • (modified) llvm/unittests/DebugInfo/BTF/CMakeLists.txt (-2)
  • (modified) llvm/unittests/DebugInfo/CodeView/CMakeLists.txt (-2)
  • (modified) llvm/unittests/DebugInfo/DWARF/CMakeLists.txt (-2)
  • (modified) llvm/unittests/DebugInfo/GSYM/CMakeLists.txt (-2)
  • (modified) llvm/unittests/DebugInfo/MSF/CMakeLists.txt (-2)
  • (modified) llvm/unittests/DebugInfo/PDB/CMakeLists.txt (-2)
  • (modified) llvm/unittests/ExecutionEngine/CMakeLists.txt (-2)
  • (modified) llvm/unittests/ExecutionEngine/JITLink/CMakeLists.txt (-2)
  • (modified) llvm/unittests/ExecutionEngine/MCJIT/CMakeLists.txt (-2)
  • (modified) llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Support/CommandLineInit/CMakeLists.txt (-4)
  • (modified) llvm/unittests/Support/DynamicLibrary/CMakeLists.txt (+2-2)
  • (modified) llvm/unittests/Target/AArch64/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/AMDGPU/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/ARM/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/CMakeLists.txt (-3)
  • (modified) llvm/unittests/Target/LoongArch/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/PowerPC/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/RISCV/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/WebAssembly/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Target/X86/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Transforms/Coroutines/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Transforms/IPO/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Transforms/Scalar/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Transforms/Utils/CMakeLists.txt (-2)
  • (modified) llvm/unittests/Transforms/Vectorize/CMakeLists.txt (-2)
  • (modified) llvm/unittests/tools/llvm-cfi-verify/CMakeLists.txt (-2)
  • (modified) llvm/unittests/tools/llvm-exegesis/CMakeLists.txt (-2)
  • (modified) llvm/unittests/tools/llvm-mca/CMakeLists.txt (-2)
  • (modified) llvm/unittests/tools/llvm-profdata/CMakeLists.txt (-2)
  • (modified) llvm/unittests/tools/llvm-profgen/CMakeLists.txt (-2)
  • (modified) llvm/utils/LLVMVisualizers/CMakeLists.txt (+1-1)
  • (modified) llvm/utils/TableGen/Basic/CMakeLists.txt (-1)
  • (modified) llvm/utils/TableGen/CMakeLists.txt (-2)
  • (modified) llvm/utils/TableGen/Common/CMakeLists.txt (-1)
  • (modified) llvm/utils/lit/CMakeLists.txt (+2-2)
  • (modified) llvm/utils/llvm-locstats/CMakeLists.txt (+1-1)
  • (modified) llvm/utils/mlgo-utils/CMakeLists.txt (+1-1)
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 43181af3bc1953..48a6ab7d21f482 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -1124,7 +1124,7 @@ configure_file(
 add_custom_target(srpm
   COMMAND cpack -G TGZ --config CPackSourceConfig.cmake -B ${LLVM_SRPM_DIR}/SOURCES
   COMMAND rpmbuild -bs --define '_topdir ${LLVM_SRPM_DIR}' ${LLVM_SRPM_BINARY_SPECFILE})
-set_target_properties(srpm PROPERTIES FOLDER "Misc")
+set_target_properties(srpm PROPERTIES FOLDER "LLVM/Misc")
 
 if(APPLE AND DARWIN_LTO_LIBRARY)
   set(CMAKE_EXE_LINKER_FLAGS
@@ -1222,7 +1222,9 @@ if( LLVM_INCLUDE_UTILS )
   add_subdirectory(utils/split-file)
   add_subdirectory(utils/mlgo-utils)
   if( LLVM_INCLUDE_TESTS )
+    set(LLVM_SUBPROJECT_TITLE "Third-Party/Google Test")
     add_subdirectory(${LLVM_THIRD_PARTY_DIR}/unittest ${CMAKE_CURRENT_BINARY_DIR}/third-party/unittest)
+    set(LLVM_SUBPROJECT_TITLE) 
   endif()
 else()
   if ( LLVM_INCLUDE_TESTS )
@@ -1286,7 +1288,7 @@ if( LLVM_INCLUDE_TESTS )
   if(LLVM_ALL_LIT_DEPENDS OR LLVM_ALL_ADDITIONAL_TEST_DEPENDS)
     add_dependencies(test-depends ${LLVM_ALL_LIT_DEPENDS} ${LLVM_ALL_ADDITIONAL_TEST_DEPENDS})
   endif()
-  set_target_properties(test-depends PROPERTIES FOLDER "Tests")
+  set_target_properties(test-depends PROPERTIES FOLDER "LLVM/Tests")
   add_dependencies(check-all test-depends)
 endif()
 
@@ -1343,7 +1345,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   # Installing the headers needs to depend on generating any public
   # tablegen'd headers.
   add_custom_target(llvm-headers DEPENDS intrinsics_gen omp_gen)
-  set_target_properties(llvm-headers PROPERTIES FOLDER "Misc")
+  set_target_properties(llvm-headers PROPERTIES FOLDER "LLVM/Resources")
 
   if (NOT LLVM_ENABLE_IDE)
     add_llvm_install_targets(install-llvm-headers
@@ -1353,7 +1355,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
 
   # Custom target to install all libraries.
   add_custom_target(llvm-libraries)
-  set_target_properties(llvm-libraries PROPERTIES FOLDER "Misc")
+  set_target_properties(llvm-libraries PROPERTIES FOLDER "LLVM/Resources")
 
   if (NOT LLVM_ENABLE_IDE)
     add_llvm_install_targets(install-llvm-libraries
@@ -1399,6 +1401,8 @@ if (LLVM_INCLUDE_BENCHMARKS)
   set(HAVE_STD_REGEX ON CACHE BOOL "OK" FORCE)
   add_subdirectory(${LLVM_THIRD_PARTY_DIR}/benchmark
     ${CMAKE_CURRENT_BINARY_DIR}/third-party/benchmark)
+  set_target_properties(benchmark PROPERTIES FOLDER "Third-Party/Google Benchmark")
+  set_target_properties(benchmark_main PROPERTIES FOLDER "Third-Party/Google Benchmark")
   add_subdirectory(benchmarks)
 endif()
 
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 693fd5669f63f9..25e35e22ccb714 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -4,6 +4,21 @@ include(LLVMProcessSources)
 include(LLVM-Config)
 include(DetermineGCCCompatible)
 
+# get_subproject_title(titlevar)
+#   Set ${outvar} to the title of the current LLVM subproject (Clang, MLIR ...)
+# 
+# The title is set in the subproject's top-level using the variable
+# LLVM_SUBPROJECT_TITLE. If it does not exist, it is assumed it is LLVM itself.
+# The title is not semantically significant, but use to create folders in
+# CMake-generated IDE projects (Visual Studio/XCode).
+function(get_subproject_title outvar)
+  if (LLVM_SUBPROJECT_TITLE)
+    set(${outvar} "${LLVM_SUBPROJECT_TITLE}" PARENT_SCOPE) 
+  else ()
+    set(${outvar} "LLVM" PARENT_SCOPE)
+  endif ()
+endfunction(get_subproject_title)
+
 function(llvm_update_compile_flags name)
   get_property(sources TARGET ${name} PROPERTY SOURCES)
   if("${sources}" MATCHES "\\.c(;|$)")
@@ -151,7 +166,8 @@ function(add_llvm_symbol_exports target_name export_file)
   endif()
 
   add_custom_target(${target_name}_exports DEPENDS ${native_export_file})
-  set_target_properties(${target_name}_exports PROPERTIES FOLDER "Misc")
+  get_subproject_title(subproject_title)
+  set_target_properties(${target_name}_exports PROPERTIES FOLDER "${subproject_title}/Misc")
 
   get_property(srcs TARGET ${target_name} PROPERTY SOURCES)
   foreach(src ${srcs})
@@ -543,6 +559,8 @@ function(llvm_add_library name)
     endif()
   endif()
 
+  get_subproject_title(subproject_title)
+
   # Generate objlib
   if((ARG_SHARED AND ARG_STATIC) OR ARG_OBJECT)
     # Generate an obj library for both targets.
@@ -564,7 +582,7 @@ function(llvm_add_library name)
     # Bring in the target include directories from our original target.
     target_include_directories(${obj_name} PRIVATE $<TARGET_PROPERTY:${name},INCLUDE_DIRECTORIES>)
 
-    set_target_properties(${obj_name} PROPERTIES FOLDER "Object Libraries")
+    set_target_properties(${obj_name} PROPERTIES FOLDER "${subproject_title}/Object Libraries")
     if(ARG_DEPENDS)
       add_dependencies(${obj_name} ${ARG_DEPENDS})
     endif()
@@ -603,6 +621,7 @@ function(llvm_add_library name)
       LINK_LIBS ${ARG_LINK_LIBS}
       LINK_COMPONENTS ${ARG_LINK_COMPONENTS}
       )
+    set_target_properties(${name_static} PROPERTIES FOLDER "${subproject_title}/Libraries")
 
     # Bring in the target link info from our original target.
     target_link_directories(${name_static} PRIVATE $<TARGET_PROPERTY:${name},LINK_DIRECTORIES>)
@@ -620,6 +639,7 @@ function(llvm_add_library name)
   else()
     add_library(${name} STATIC ${ALL_FILES})
   endif()
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Libraries")
 
   if(ARG_COMPONENT_LIB)
     set_target_properties(${name} PROPERTIES LLVM_COMPONENT TRUE)
@@ -796,6 +816,8 @@ function(add_llvm_install_targets target)
     endif()
   endforeach()
 
+  get_subproject_title(subproject_title)
+
   add_custom_target(${target}
                     DEPENDS ${file_dependencies}
                     COMMAND "${CMAKE_COMMAND}"
@@ -803,7 +825,7 @@ function(add_llvm_install_targets target)
                             ${prefix_option}
                             -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
                     USES_TERMINAL)
-  set_target_properties(${target} PROPERTIES FOLDER "Component Install Targets")
+  set_target_properties(${target} PROPERTIES FOLDER "${subproject_title}/Install")
   add_custom_target(${target}-stripped
                     DEPENDS ${file_dependencies}
                     COMMAND "${CMAKE_COMMAND}"
@@ -812,7 +834,7 @@ function(add_llvm_install_targets target)
                             -DCMAKE_INSTALL_DO_STRIP=1
                             -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
                     USES_TERMINAL)
-  set_target_properties(${target}-stripped PROPERTIES FOLDER "Component Install Targets (Stripped)")
+  set_target_properties(${target}-stripped PROPERTIES FOLDER "${subproject_title}/Install")
   if(target_dependencies)
     add_dependencies(${target} ${target_dependencies})
     add_dependencies(${target}-stripped ${target_dependencies})
@@ -832,6 +854,8 @@ endfunction()
 function(add_llvm_component_group name)
   cmake_parse_arguments(ARG "HAS_JIT" "" "LINK_COMPONENTS" ${ARGN})
   add_custom_target(${name})
+  get_subproject_title(subproject_title)
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Component Groups")
   if(ARG_HAS_JIT)
     set_property(TARGET ${name} PROPERTY COMPONENT_HAS_JIT ON)
   endif()
@@ -865,6 +889,8 @@ function(add_llvm_component_library name)
 
   if(ARG_ADD_TO_COMPONENT)
     set_property(TARGET ${ARG_ADD_TO_COMPONENT} APPEND PROPERTY LLVM_LINK_COMPONENTS ${component_name})
+    get_subproject_title(subproject_title)
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Libraries/${ARG_ADD_TO_COMPONENT}")
   endif()
 
 endfunction()
@@ -921,10 +947,12 @@ macro(add_llvm_library name)
     endif()
     set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
   endif()
+
+  get_subproject_title(subproject_title)
   if (ARG_MODULE)
-    set_target_properties(${name} PROPERTIES FOLDER "Loadable modules")
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Loadable Modules")
   else()
-    set_target_properties(${name} PROPERTIES FOLDER "Libraries")
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Libraries")
   endif()
 endmacro(add_llvm_library name)
 
@@ -948,7 +976,8 @@ macro(generate_llvm_objects name)
       add_dependencies(${obj_name} ${ARG_DEPENDS})
     endif()
 
-    set_target_properties(${obj_name} PROPERTIES FOLDER "Object Libraries")
+    get_subproject_title(subproject_title)
+    set_target_properties(${obj_name} PROPERTIES FOLDER "${subproject_title}/Object Libraries")
   endif()
 
   if (ARG_GENERATE_DRIVER)
@@ -999,6 +1028,8 @@ macro(add_llvm_executable name)
   else()
     add_executable(${name} ${ALL_FILES})
   endif()
+  get_subproject_title(subproject_title)
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Executables")
 
   setup_dependency_debugging(${name} ${LLVM_COMMON_DEPENDS})
 
@@ -1418,8 +1449,9 @@ macro(llvm_add_tool project name)
     if( LLVM_BUILD_TOOLS )
       set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
     endif()
-    set_target_properties(${name} PROPERTIES FOLDER "Tools")
+    get_subproject_title(subproject_title)
   endif()
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Tools")
 endmacro(llvm_add_tool project name)
 
 macro(add_llvm_tool name)
@@ -1435,7 +1467,8 @@ macro(add_llvm_example name)
   if( LLVM_BUILD_EXAMPLES )
     install(TARGETS ${name} RUNTIME DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}")
   endif()
-  set_target_properties(${name} PROPERTIES FOLDER "Examples")
+  get_subproject_title(subproject_title)
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Examples")
 endmacro(add_llvm_example name)
 
 macro(add_llvm_example_library name)
@@ -1446,7 +1479,8 @@ macro(add_llvm_example_library name)
     add_llvm_library(${name} ${ARGN})
   endif()
 
-  set_target_properties(${name} PROPERTIES FOLDER "Examples")
+  get_subproject_title(subproject_title)
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Examples")
 endmacro(add_llvm_example_library name)
 
 # This is a macro that is used to create targets for executables that are needed
@@ -1457,7 +1491,8 @@ macro(add_llvm_utility name)
   endif()
 
   add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
-  set_target_properties(${name} PROPERTIES FOLDER "Utils")
+  get_subproject_title(subproject_title)
+  set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Utils")
   if ( ${name} IN_LIST LLVM_TOOLCHAIN_UTILITIES OR NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
     if (LLVM_INSTALL_UTILS AND LLVM_BUILD_UTILS)
       get_target_export_arg(${name} LLVM export_to_llvmexports)
@@ -1480,19 +1515,20 @@ endmacro(add_llvm_utility name)
 
 macro(add_llvm_fuzzer name)
   cmake_parse_arguments(ARG "" "DUMMY_MAIN" "" ${ARGN})
+  get_subproject_title(subproject_title)
   if( LLVM_LIB_FUZZING_ENGINE )
     set(LLVM_OPTIONAL_SOURCES ${ARG_DUMMY_MAIN})
     add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
     target_link_libraries(${name} PRIVATE ${LLVM_LIB_FUZZING_ENGINE})
-    set_target_properties(${name} PROPERTIES FOLDER "Fuzzers")
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Fuzzers")
   elseif( LLVM_USE_SANITIZE_COVERAGE )
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
     set(LLVM_OPTIONAL_SOURCES ${ARG_DUMMY_MAIN})
     add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
-    set_target_properties(${name} PROPERTIES FOLDER "Fuzzers")
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Fuzzers")
   elseif( ARG_DUMMY_MAIN )
     add_llvm_executable(${name} ${ARG_DUMMY_MAIN} ${ARG_UNPARSED_ARGUMENTS})
-    set_target_properties(${name} PROPERTIES FOLDER "Fuzzers")
+    set_target_properties(${name} PROPERTIES FOLDER "${subproject_title}/Fuzzers")
   endif()
 endmacro()
 
@@ -1653,6 +1689,8 @@ function(add_unittest test_suite test_name)
 
   list(APPEND LLVM_LINK_COMPONENTS Support) # gtest needs it for raw_ostream
   add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO NO_INSTALL_RPATH ${ARGN})
+  get_subproject_title(subproject_title)
+  set_target_properties(${test_name} PROPERTIES FOLDER "${subproject_title}/Tests/Unit")
 
   # The runtime benefits of LTO don't outweight the compile time costs for tests.
   if(LLVM_ENABLE_LTO)
@@ -1684,10 +1722,6 @@ function(add_unittest test_suite test_name)
   target_link_libraries(${test_name} PRIVATE llvm_gtest_main llvm_gtest ${LLVM_PTHREAD_LIB})
 
   add_dependencies(${test_suite} ${test_name})
-  get_target_property(test_suite_folder ${test_suite} FOLDER)
-  if (test_suite_folder)
-    set_property(TARGET ${test_name} PROPERTY FOLDER "${test_suite_folder}")
-  endif ()
 endfunction()
 
 # Use for test binaries that call llvm::getInputFileDirectory(). Use of this
@@ -1710,7 +1744,8 @@ function(add_benchmark benchmark_name)
   add_llvm_executable(${benchmark_name} IGNORE_EXTERNALIZE_DEBUGINFO NO_INSTALL_RPATH ${ARGN})
   set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
   set_output_directory(${benchmark_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
-  set_property(TARGET ${benchmark_name} PROPERTY FOLDER "Utils")
+  get_subproject_title(subproject_title)
+  set_property(TARGET ${benchmark_name} PROPERTY FOLDER "${subproject_title}/Utils")
   target_link_libraries(${benchmark_name} PRIVATE benchmark)
 endfunction()
 
@@ -1999,6 +2034,8 @@ function(add_lit_target target comment)
       COMMAND ${CMAKE_COMMAND} -E echo "${target} does nothing, no tools built.")
     message(STATUS "${target} does nothing.")
   endif()
+  get_subproject_title(subproject_title)
+  set_target_properties(${target} PROPERTIES FOLDER "${subproject_title}/Tests")
 
   if (ARG_DEPENDS)
     add_dependencies(${target} ${ARG_DEPENDS})
@@ -2080,7 +2117,8 @@ function(add_lit_testsuites project directory)
     cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "FOLDER" "PARAMS;DEPENDS;ARGS" ${ARGN})
 
     if (NOT ARG_FOLDER)
-      set(ARG_FOLDER "Test Subdirectories")
+      get_subproject_title(subproject_title)
+      set(ARG_FOLDER "${subproject_title}/Tests")
     endif()
 
     # Search recursively for test directories by assuming anything not
@@ -2282,7 +2320,8 @@ function(llvm_add_tool_symlink project link_name target)
       set(should_build_all ALL)
     endif()
     add_custom_target(${target_name} ${should_build_all} DEPENDS ${target} ${output_path})
-    set_target_properties(${target_name} PROPERTIES FOLDER Tools)
+    get_subproject_title(subproject_title)
+    set_target_properties(${target_name} PROPERTIES FOLDER "${subproject_title}/Tools")
 
     # Make sure both the link and target are toolchain tools
     if (${link_name} IN_LIST LLVM_TOOLCHAIN_TOOLS AND ${target} IN_LIST LLVM_TOOLCHAIN_TOOLS)
@@ -2542,5 +2581,7 @@ function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
   if(LLVM_USE_HOST_TOOLS AND NOT ${setting_name})
     build_native_tool(${tool_name} exe_name DEPENDS ${tool_name})
     add_custom_target(${target_var_name} DEPENDS ${exe_name})
+    get_subproject_title(subproject_title)
+    set_target_properties(${target_var_name} PROPERTIES FOLDER "${subproject_title}/Native")
   endif()
 endfunction()
diff --git a/llvm/cmake/modules/AddOCaml.cmake b/llvm/cmake/modules/AddOCaml.cmake
index 891c9e6d618c06..01e736a68c3be2 100644
--- a/llvm/cmake/modules/AddOCaml.cmake
+++ b/llvm/cmake/modules/AddOCaml.cmake
@@ -173,6 +173,7 @@ function(add_ocaml_library name)
     VERBATIM)
 
   add_custom_target("ocaml_${name}" ALL DEPENDS ${ocaml_outputs} "${bin}/${name}.odoc")
+  set_target_properties("ocaml_${name}" PROPERTIES FOLDER "LLVM/Bindings/OCaml")
 
   set_target_properties("ocaml_${name}" PROPERTIES
     OCAML_FLAGS "-I;${bin}")
@@ -228,5 +229,5 @@ endfunction()
 add_custom_target(ocaml_make_directory
   COMMAND "${CMAKE_COMMAND}" "-E" "make_directory" "${LLVM_LIBRARY_DIR}/ocaml/llvm")
 add_custom_target("ocaml_all")
-set_target_properties(ocaml_all PROPERTIES FOLDER "Misc")
-set_target_properties(ocaml_make_directory PROPERTIES FOLDER "Misc")
+set_target_properties(ocaml_all PROPERTIES FOLDER "LLVM/Bindings/OCaml")
+set_target_properties(ocaml_make_directory PROPERTIES FOLDER "LLVM/Bindings/OCaml")
diff --git a/llvm/cmake/modules/AddSphinxTarget.cmake b/llvm/cmake/modules/AddSphinxTarget.cmake
index b90639fbbf073b..9de169d7297cb7 100644
--- a/llvm/cmake/modules/AddSphinxTarget.cmake
+++ b/llvm/cmake/modules/AddSphinxTarget.cmake
@@ -6,6 +6,7 @@ if (LLVM_ENABLE_SPHINX)
   find_package(Sphinx REQUIRED)
   if (LLVM_BUILD_DOCS AND NOT TARGET sphinx)
     add_custom_target(sphinx ALL)
+    set_target_properties(sphinx PROPERTIES FOLDER "LLVM/Docs")
   endif()
 else()
   message(STATUS "Sphinx disabled.")
@@ -58,6 +59,8 @@ function (add_sphinx_target builder project)
                             "${SPHINX_BUILD_DIR}" # Output
                     COMMENT
                     "Generating ${builder} Sphinx documentation for ${project} into \"${SPHINX_BUILD_DIR}\"")
+  get_subproject_title(subproject_title)
+  set_target_properties(${SPHINX_TARGET_NAME} PROPERTIES FOLDER "${subproject_title}/Docs")
 
   # When "clean" target is run, remove the Sphinx build directory
   set_property(DIRECTORY APPEND PROPERTY
diff --git a/llvm/cmake/modules/CrossCompile.cmake b/llvm/cmake/modules/CrossCompile.cmake
index 55bf3be756427c..39b4abaa0d9313 100644
--- a/llvm/cmake/modules/CrossCompile.cmake
+++ b/llvm/cmake/modules/CrossCompile.cmake
@@ -45,6 +45,8 @@ function(llvm_create_cross_target project_name target_name toolchain buildtype)
 
   add_custom_target(CREATE_${project_name}_${target_name}
     DEPENDS ${${project_name}_${target_name}_BUILD})
+  get_subproject_title(subproject_title)
+  set_target_properties(CREATE_${project_name}_${target_name} PROPERTIES FOLDER "${subproject_title}/Native")
 
   # Escape semicolons in the targets list so that cmake doesn't expand
   # them to spaces.
@@ -98,6 +100,8 @@ function(llvm_create_cross_target project_name target_name toolchain buildtype)
 
   add_custom_target(CONFIGURE_${project_name}_${target_name}
     DEPENDS ${${project_name}_${target_name}_BUILD}/CMakeCache.txt)
+  get_subproject_title(subproject_title)
+  set_target_properties(CONFIGURE_${project_name}_${target_name} PROPERTIES FOLDER "${subproject_title}/Native")
 
 endfunction()
 
diff --git a/llvm/cmake/modules/LLVMDistributionSupport.cmake b/llvm/cmake/modules/LLVMDistributionSupport.cmake
index 0b78f8f9137c5a..011548419e0c32 100644
--- a/llvm/cmake/modules/LLVMDistributionSupport.cmake
+++ b/llvm/cmake/modules/LLVMDistributionSupport.cmake
@@ -210,6 +210,7 @@ function(install_distribution_exports project)
                 COMPONENT ${target})
         if(NOT LLVM_ENABLE_IDE)
           add_custom_target(${target})
+          set_target_properties(${target} PROPERTIES FOLDER "LLVM/Distribution")
           add_llvm_install_targets(install-${target} COMPONENT ${target})
         endif()
       endif()
@@ -260,6 +261,13 @@ function(llvm_distribution_add_targets)
     add_custom_target(${distribution_target})
     add_custom_target(install-${distribution_target})
     add_custom_target(install-${distribution_target}-stripped)
+    set_target_properties(
+        ${distribution_target} 
+        install-${distribution_target}
+        install-${distribution_target}-stripped
+      PROPERTIES 
+        FOLDER "LLVM/Distribution"
+    )
 
     foreach(target ${distribution_components})
       # Note that some distribution components may not have an actual target, but only an install-FOO target.
diff --git a/llvm/cmake/modules/LLVMExternalProjectUtils.cmake b/llvm/cmake/modules/LLVMExternalProjectUtils.cmake
index 2672f90f579bd9..edfee259470df5 100644
--- a/llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ b/llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -56,11 +56,13 @@ endfunction()
 #     Use provided strip tool instead of the default one.
 #   TARGET_TRIPLE triple
 #     Optional target triple to pass to the compiler
+#   FOLDER
+#     For IDEs, the Folder to put the targets into.
 #   )
 function(llvm_ExternalProject_Add name source_dir)
   cmake_parse_arguments(ARG
     "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL;ALWAYS_CLEAN"
-    "SOURCE_DIR"
+    "SOURCE_DIR;FOLDER"
     "CMAKE_ARGS;TOOLCHAIN_TOOLS;RUNTIME_LIBRARIES;DEPENDS;EXTRA_TARGETS;PASSTHROUGH_PREF...
[truncated]

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a benefit in renaming folders like "Tools" and "Tests" to "LLVM Tests" and "LLVM Tools", but I really don't think having folders within folders is a good UI for regular projects: it'll mean extra clicking in the solution explorer just to get to the file you want (a process which is already a bit suboptimal as it is), for no real added benefit when you can just make the folders top-level and named to distinguish subprojects.

llvm/unittests/DebugInfo/BTF/CMakeLists.txt Show resolved Hide resolved
llvm/unittests/Analysis/InlineOrderPlugin/CMakeLists.txt Outdated Show resolved Hide resolved
@Meinersbur
Copy link
Member Author

I see a benefit in renaming folders like "Tools" and "Tests" to "LLVM Tests" and "LLVM Tools", but I really don't think having folders within folders is a good UI for regular projects: it'll mean extra clicking in the solution explorer just to get to the file you want (a process which is already a bit suboptimal as it is), for no real added benefit when you can just make the folders top-level and named to distinguish subprojects.

Having the suprojects in subfolders allows you to unload these projects from the VS IDE solution (including LLVM itself) without re-running cmake. It remembers which folders are expanded or collapsed after reloading or even rerunning cmake so if you prefer subprojects expanded there is only one additional click per subproject you will ever need to do. Usually, you only expand those you are actually working on in which case this means less clutter in the project view.

@jh7370
Copy link
Collaborator

jh7370 commented Apr 25, 2024

I see a benefit in renaming folders like "Tools" and "Tests" to "LLVM Tests" and "LLVM Tools", but I really don't think having folders within folders is a good UI for regular projects: it'll mean extra clicking in the solution explorer just to get to the file you want (a process which is already a bit suboptimal as it is), for no real added benefit when you can just make the folders top-level and named to distinguish subprojects.

Having the suprojects in subfolders allows you to unload these projects from the VS IDE solution (including LLVM itself) without re-running cmake.

When you say "unload these projects" are you referring to effectively disabling them via the "Unload solution projects in folder" option? If so, you don't need them in a subfolder to achive this: just select multiple folders and click "Unload project". So, if you named all the, e.g. clang folders Clang ... they'll be trivially easy to multi-select, since they'll be in order.

It remembers which folders are expanded or collapsed after reloading or even rerunning cmake so if you prefer subprojects expanded there is only one additional click per subproject you will ever need to do. Usually, you only expand those you are actually working on in which case this means less clutter in the project view.

As someone who routinely works in projects similar to LLVM where the solution uses cmake, but not the built-in VS variation, I agree that it does appear to normally remember the expansion/collapse state, but I've also seen many instances where it doesn't, especially where projects need reloading following an implicit cmake run due to a CMakeLists.txt file change. It also doesn't account for the fact that I routinely expand and collapse folders, so that I can easily see the projects I'm currently working on (often I am working on multiple projects in different folders at the same time in my local codebase). This is especially significant for solutions with many projects like LLVM, where they take up more screen space in the Solution Explorer, and worse when using a small screen like a laptop. So, if this change were to land as-is, with additional layers of subfolders, it would make my experience working with LLVM worse, not better (note: I am strongly in favour of rationalising the organisation as you're attempting, just not with subdirectories).

@Meinersbur
Copy link
Member Author

I see a benefit in renaming folders like "Tools" and "Tests" to "LLVM Tests" and "LLVM Tools", but I really don't think having folders within folders is a good UI for regular projects: it'll mean extra clicking in the solution explorer just to get to the file you want (a process which is already a bit suboptimal as it is), for no real added benefit when you can just make the folders top-level and named to distinguish subprojects.

Having the suprojects in subfolders allows you to unload these projects from the VS IDE solution (including LLVM itself) without re-running cmake.

When you say "unload these projects" are you referring to effectively disabling them via the "Unload solution projects in folder" option? If so, you don't need them in a subfolder to achive this: just select multiple folders and click "Unload project". So, if you named all the, e.g. clang folders Clang ... they'll be trivially easy to multi-select, since they'll be in order.

It'll mean extra clicking. Which also seems to be your argument against it, just in different places.

In case of LLVM there are 18 subfolders. I really want them to be collapsable to not take more clutter in the projects view then necessary when I don't use them.
image

It also doesn't account for the fact that I routinely expand and collapse folders, so that I can easily see the projects I'm currently working on (often I am working on multiple projects in different folders at the same time in my local codebase). This is especially significant for solutions with many projects like LLVM, where they take up more screen space in the Solution Explorer, and worse when using a small screen like a laptop.

This sounds like an argument in favor of subproject-folders to me. It allows you to expand/collapse on more granularities and collapsed folders take less space.

If you just want to see the subproject name in the Executables/Libraries/... folder, could we compromise on this?
image

@jh7370
Copy link
Collaborator

jh7370 commented Apr 29, 2024

Backing up a moment - how many subfolders do you expect inside each top-level folder?

@Meinersbur
Copy link
Member Author

Backing up a moment - how many subfolders do you expect inside each top-level folder?

Exactly as shown. LLVM is the project with the most subfolders. There may be more in the future, depending on whether there are suitable categories.

@jh7370
Copy link
Collaborator

jh7370 commented Apr 30, 2024

Backing up a moment - how many subfolders do you expect inside each top-level folder?

Exactly as shown. LLVM is the project with the most subfolders. There may be more in the future, depending on whether there are suitable categories.

Sorry, I should have been clearer that I meant in general, not specifically LLVM (i.e. including clang/lld/lldb/...), ideally broken down by subproject. It helps give a clearer indication as to the overall impact and the trade-offs between subfolders and screen real estate (which I acknowledge is a potential issue).

@Meinersbur
Copy link
Member Author

image

Even though I have a 4K screen, it doesn't fit. This was created using image editing.

@jh7370
Copy link
Collaborator

jh7370 commented Apr 30, 2024

Okay, I'm going to take a step back from reviewing this - I'm no longer opposed to the subfolders, but I also don't have enough knowledge of the different LLVM components to be able to reasonably review whether any given folder is the right place for a project.

@Meinersbur
Copy link
Member Author

I don't think it is necessary to get every single folder right immediately. The folder structure was not always cared for so needed some maintenance and this is also trying to make future maintenance easier. For instance, Compiler-RT put everything into "Compiler-RT Misc". It should be sufficient if there is a clear improvement. I don't think there is anybody who knows what every single CMake target is for.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the folder structure looks reasonable in Visual Studio 2022; LGTM!

@Meinersbur Meinersbur merged commit 4ecbfac into main May 25, 2024
4 of 5 checks passed
@Meinersbur Meinersbur deleted the users/meinersbur/ide_folders_llvm branch May 25, 2024 11:28
@Meinersbur Meinersbur restored the users/meinersbur/ide_folders_llvm branch May 25, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants