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

Revise IDE folder structure #89755

Merged
merged 10 commits into from
May 25, 2024
Merged

Revise IDE folder structure #89755

merged 10 commits into from
May 25, 2024

Conversation

Meinersbur
Copy link
Member

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the part for the remaining subprojects that have just one file changed each. For the runtime subprojects this is because they cannot be used in LLVM_ENABLE_PROJECTS and therefore its targets do not appear in the IDE when compiling as part of LLVM. LLVM_SUBPROJECT_TITLE is still defined and useful when loading the project from the runtimes/runtimes-bins folder withing the LLVM build dir, or for out-of-tree builds. A follow-up patch may do a more complete folder organization. See #89153 for the entire series and 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.

@Meinersbur Meinersbur mentioned this pull request Apr 23, 2024
@Meinersbur Meinersbur marked this pull request as ready for review May 21, 2024 15:54
@Meinersbur Meinersbur requested review from a team as code owners May 21, 2024 15:54
@llvmbot llvmbot added cmake Build system in general and CMake in particular libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. libunwind libc offload labels May 21, 2024
@llvmbot
Copy link

llvmbot commented May 21, 2024

@llvm/pr-subscribers-libunwind
@llvm/pr-subscribers-libc
@llvm/pr-subscribers-offload
@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: Michael Kruse (Meinersbur)

Changes

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the part for the remaining subprojects that have just one file changed each. For the runtime subprojects this is because they cannot be used in LLVM_ENABLE_PROJECTS and therefore its targets do not appear in the IDE when compiling as part of LLVM. LLVM_SUBPROJECT_TITLE is still defined and useful when loading the project from the runtimes/runtimes-bins folder withing the LLVM build dir, or for out-of-tree builds. A follow-up patch may do a more complete folder organization. See #89153 for the entire series and 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.

Full diff: https://github.com/llvm/llvm-project/pull/89755.diff

9 Files Affected:

  • (modified) cross-project-tests/CMakeLists.txt (+1-5)
  • (modified) libc/CMakeLists.txt (+1)
  • (modified) libcxx/CMakeLists.txt (+1)
  • (modified) libcxxabi/CMakeLists.txt (+1)
  • (modified) libunwind/CMakeLists.txt (+1)
  • (modified) llvm-libgcc/CMakeLists.txt (+1)
  • (modified) offload/CMakeLists.txt (+1)
  • (modified) pstl/CMakeLists.txt (+1)
  • (modified) runtimes/CMakeLists.txt (+1)
diff --git a/cross-project-tests/CMakeLists.txt b/cross-project-tests/CMakeLists.txt
index f7c2ca7ad83de..7f2fee48fda77 100644
--- a/cross-project-tests/CMakeLists.txt
+++ b/cross-project-tests/CMakeLists.txt
@@ -3,6 +3,7 @@
 # The subset inside debuginfo-tests invoke clang to generate programs with
 # various types of debug info, and then run those programs under a debugger
 # such as GDB or LLDB to verify the results.
+set(LLVM_SUBPROJECT_TITLE "Cross-Project")
 
 find_package(Python3 COMPONENTS Interpreter)
 
@@ -97,8 +98,3 @@ add_lit_testsuite(check-cross-amdgpu "Running AMDGPU cross-project tests"
 add_lit_testsuites(CROSS_PROJECT ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CROSS_PROJECT_TEST_DEPS}
   )
-
-set_target_properties(check-cross-project PROPERTIES FOLDER "Tests")
-set_target_properties(check-debuginfo PROPERTIES FOLDER "Tests")
-set_target_properties(check-intrinsic-headers PROPERTIES FOLDER "Tests")
-set_target_properties(check-cross-amdgpu PROPERTIES FOLDER "Tests")
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 175efd89d67e6..f35471a06a53e 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -1,4 +1,5 @@
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "libc")
 
 # Include LLVM's cmake policies.
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 2977c26646cb2..d75f22ecf0222 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -5,6 +5,7 @@
 # Setup Project
 #===============================================================================
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "libc++")
 
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index da998d2221dc4..f7673da25d20e 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -5,6 +5,7 @@
 #===============================================================================
 
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "libc++abi")
 
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 806d5a783ec39..2117cd9e756ef 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -3,6 +3,7 @@
 #===============================================================================
 
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "libunwind")
 
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
diff --git a/llvm-libgcc/CMakeLists.txt b/llvm-libgcc/CMakeLists.txt
index 013c9ca2e3307..c6641ab9e3219 100644
--- a/llvm-libgcc/CMakeLists.txt
+++ b/llvm-libgcc/CMakeLists.txt
@@ -3,6 +3,7 @@
 #===============================================================================
 
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "LLVM libgcc")
 
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index abc8baa0805ff..44a3d17b2e59a 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -11,6 +11,7 @@
 ##===----------------------------------------------------------------------===##
 
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "liboffload")
 
 if ("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
   set(OPENMP_STANDALONE_BUILD TRUE)
diff --git a/pstl/CMakeLists.txt b/pstl/CMakeLists.txt
index 255e22af9a26b..592e11d356473 100644
--- a/pstl/CMakeLists.txt
+++ b/pstl/CMakeLists.txt
@@ -6,6 +6,7 @@
 #
 #===----------------------------------------------------------------------===##
 cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "Parallel STL")
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index fcc59c8fa1c37..76ad8aa1f353e 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -9,6 +9,7 @@ include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
 include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
 
 project(Runtimes C CXX ASM)
+set(LLVM_SUBPROJECT_TITLE "Runtimes")
 
 list(INSERT CMAKE_MODULE_PATH 0
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

libc parts LGTM

@@ -9,6 +9,7 @@ include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)

project(Runtimes C CXX ASM)
set(LLVM_SUBPROJECT_TITLE "Runtimes")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? My understanding is that the runtimes CMake here is the root that includes all the other projects, so this doesn't actually contain any "projects'>

Copy link
Member Author

Choose a reason for hiding this comment

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

Since LLVM is usually the top-level project everything is sorted under "LLVM" be default. I added a LLVM_SUBPROJECT_TITLE to all potential subprojects that are either included via add_subdirectory or as a stand-alone build, including this one.

I just checked to see if it is actually used, and the answer is yes: At least check-runtimes is put into this folder when loading the solution from the runtimes-bins directory.

I did not go through all runtime targets to ensure they are put into some folder name that makes sense like I did for the LLVM_ENABLE_PROJECTS subprojects since opening a project in the IDE from the runtimes-bin would be done only if there is a concrete problem. However, I wanted to at least make a minimal effort that allows puts targets other than add_custom_target into a subtproject folder.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM. After checking, I think this only impacts add_lit_testsuites since we don't use many other things from AddLLVM.cmake in the runtimes.

@Meinersbur Meinersbur deleted the branch llvm:main May 25, 2024 11:28
@Meinersbur Meinersbur closed this May 25, 2024
@Meinersbur Meinersbur reopened this May 25, 2024
@Meinersbur Meinersbur changed the base branch from users/meinersbur/ide_folders_llvm to main May 25, 2024 15:40
@Meinersbur Meinersbur merged commit e14f5f2 into llvm:main May 25, 2024
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc libunwind offload
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants