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

[BOLT] Revise IDE folder structure #89742

Merged
merged 11 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 BOLT part. 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 April 24, 2024 11:31
Copy link
Contributor

@rafaelauler rafaelauler left a comment

Choose a reason for hiding this comment

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

LGTM

@llvmbot llvmbot added the BOLT label May 21, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 21, 2024

@llvm/pr-subscribers-bolt

Author: Michael Kruse (Meinersbur)

Changes

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the BOLT part. 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/89742.diff

5 Files Affected:

  • (modified) bolt/CMakeLists.txt (+3-1)
  • (modified) bolt/cmake/modules/AddBOLT.cmake (-1)
  • (modified) bolt/docs/CMakeLists.txt (+1)
  • (modified) bolt/test/CMakeLists.txt (+1-2)
  • (modified) bolt/unittests/CMakeLists.txt (+1-1)
diff --git a/bolt/CMakeLists.txt b/bolt/CMakeLists.txt
index cc3a70fa35e0a..3c1fe967d6597 100644
--- a/bolt/CMakeLists.txt
+++ b/bolt/CMakeLists.txt
@@ -1,3 +1,5 @@
+set(LLVM_SUBPROJECT_TITLE "BOLT")
+
 include(ExternalProject)
 
 set(BOLT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
@@ -121,7 +123,7 @@ option(BOLT_BUILD_TOOLS
   "Build the BOLT tools. If OFF, just generate build targets." ON)
 
 add_custom_target(bolt)
-set_target_properties(bolt PROPERTIES FOLDER "BOLT")
+set_target_properties(bolt PROPERTIES FOLDER "BOLT/Meta")
 add_llvm_install_targets(install-bolt DEPENDS bolt COMPONENT bolt)
 
 include_directories(
diff --git a/bolt/cmake/modules/AddBOLT.cmake b/bolt/cmake/modules/AddBOLT.cmake
index 1f69b9046320a..c7ac662c6b121 100644
--- a/bolt/cmake/modules/AddBOLT.cmake
+++ b/bolt/cmake/modules/AddBOLT.cmake
@@ -3,7 +3,6 @@ include(LLVMDistributionSupport)
 
 macro(add_bolt_executable name)
   add_llvm_executable(${name} ${ARGN})
-  set_target_properties(${name} PROPERTIES FOLDER "BOLT")
 endmacro()
 
 macro(add_bolt_tool name)
diff --git a/bolt/docs/CMakeLists.txt b/bolt/docs/CMakeLists.txt
index b230512fe5717..12ae852566785 100644
--- a/bolt/docs/CMakeLists.txt
+++ b/bolt/docs/CMakeLists.txt
@@ -79,6 +79,7 @@ if (LLVM_ENABLE_DOXYGEN)
     COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
     COMMENT "Generating bolt doxygen documentation." VERBATIM)
+  set_target_properties(doxygen-bolt PROPERTIES FOLDER "BOLT/Docs")
 
   if (LLVM_BUILD_DOCS)
     add_dependencies(doxygen doxygen-bolt)
diff --git a/bolt/test/CMakeLists.txt b/bolt/test/CMakeLists.txt
index 89862fd59eb8e..d468ff984840f 100644
--- a/bolt/test/CMakeLists.txt
+++ b/bolt/test/CMakeLists.txt
@@ -56,7 +56,7 @@ list(APPEND BOLT_TEST_DEPS
   )
 
 add_custom_target(bolt-test-depends DEPENDS ${BOLT_TEST_DEPS})
-set_target_properties(bolt-test-depends PROPERTIES FOLDER "BOLT")
+set_target_properties(bolt-test-depends PROPERTIES FOLDER "BOLT/Tests")
 
 add_lit_testsuite(check-bolt "Running the BOLT regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
@@ -64,7 +64,6 @@ add_lit_testsuite(check-bolt "Running the BOLT regression tests"
   DEPENDS ${BOLT_TEST_DEPS}
   ARGS ${BOLT_TEST_EXTRA_ARGS}
   )
-set_target_properties(check-bolt PROPERTIES FOLDER "BOLT")
 
 add_lit_testsuites(BOLT ${CMAKE_CURRENT_SOURCE_DIR}
   PARAMS ${BOLT_TEST_PARAMS}
diff --git a/bolt/unittests/CMakeLists.txt b/bolt/unittests/CMakeLists.txt
index 77159e92dec55..64414b83d39fe 100644
--- a/bolt/unittests/CMakeLists.txt
+++ b/bolt/unittests/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(BoltUnitTests)
-set_target_properties(BoltUnitTests PROPERTIES FOLDER "BOLT tests")
+set_target_properties(BoltUnitTests PROPERTIES FOLDER "BOLT/Tests")
 
 function(add_bolt_unittest test_dirname)
   add_unittest(BoltUnitTests ${test_dirname} ${ARGN})

@@ -121,7 +123,7 @@ option(BOLT_BUILD_TOOLS
"Build the BOLT tools. If OFF, just generate build targets." ON)

add_custom_target(bolt)
set_target_properties(bolt PROPERTIES FOLDER "BOLT")
set_target_properties(bolt PROPERTIES FOLDER "BOLT/Meta")
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably should be something more neutral then a company name.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not supposed to be the company name, but meta in the sense of self-referential. That is, targets that only exists to trigger the build of other targets. In this case, all of BOLT (as in make all if BOLT was its own build system). "Metatarget" would disambiguate the meaning.

An alternative could be "Phony".

@aaupov
Copy link
Contributor

aaupov commented May 21, 2024

Please use [BOLT] in the title.

@Meinersbur Meinersbur changed the title [bolt] Revise IDE folder structure [BOLT] Revise IDE folder structure May 22, 2024
@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 14:55
@Meinersbur Meinersbur merged commit c5a3f66 into llvm:main May 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants