Skip to content

Update IDE Folders #89153

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

Closed
wants to merge 5 commits into from
Closed

Update IDE Folders #89153

wants to merge 5 commits into from

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Apr 17, 2024

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.

Before:

After:

Visual Studio now supports CMake projects directly, but the Visual Studio solution generator still has advantages. One is that targets including entire folders can be hidden when not needed. This removes non-relevant items form code search and does not try to rebuild them. For instance, when hiding Tablegenning and Resources targets, an iterative build can be faster than using ninja. The other one is that the organization in folders is less cluttered than the CMake targets view or the startup item view1:

Footnotes

  1. Items can be hidden from the dropdown menu, but the entire list is still flat with hundreds of executables.

@Meinersbur Meinersbur marked this pull request as ready for review April 22, 2024 18:07
@llvmbot llvmbot added cmake Build system in general and CMake in particular clang Clang issues not falling into any other category clang-tools-extra compiler-rt libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. lld lldb libunwind backend:AMDGPU backend:WebAssembly backend:X86 clangd clang-tidy clang:headers Headers provided by Clang, e.g. for intrinsics labels Apr 22, 2024
Comment on lines 59 to 60
# FOLDER
# For IDEs, the Folder to put the targets into.=
Copy link
Member

Choose a reason for hiding this comment

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

Maybe if this is only affecting IDEs, then perhaps the property's name should reflect this, i.e., IDE_FOLDER? Unfortunately, directories are also called "folders" at least on Windows, so I see a slim chance of misunderstanding this as the path from which the external project is loaded.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's already called FOLDER in https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L2080-L2083. I would prefer to keep it consistent.

@JDevlieghere
Copy link
Member

This looks like a nice improvement for folks using those generators. Even though most of these changes look straightforward, it would be a lot easier to review if this was broken up per subproject. Is there any reason that's not possible?

@jh7370
Copy link
Collaborator

jh7370 commented Apr 23, 2024

This looks like a nice improvement for folks using those generators. Even though most of these changes look straightforward, it would be a lot easier to review if this was broken up per subproject. Is there any reason that's not possible?

+1 to this: 195 files is too many to review in a single PR really, so if we have an alternative, incremental approach, we should take that. (Also big +1 to the improvement though).

@Meinersbur
Copy link
Member Author

As suggested, I split the patch into per-subproject patches:

LLVM (dependency of the other patches)
BOLT
Clang
Clang-Tools-Extra
Compiler-RT
Flang
libclc
LLD
LLDB
MLIR
OpenMP
Polly
Misc

@Meinersbur Meinersbur closed this May 25, 2024
Meinersbur added a commit that referenced this pull request Jul 18, 2024
Reviewers of #89153 suggested to break up the patch into per-subproject
patches. This is the libclc 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.
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Reviewers of #89153 suggested to break up the patch into per-subproject
patches. This is the libclc 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.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU backend:WebAssembly backend:X86 clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category clang-tidy clang-tools-extra clangd cmake Build system in general and CMake in particular compiler-rt:asan Address sanitizer compiler-rt:builtins compiler-rt:cfi Control Flow Integrity compiler-rt:fuzzer compiler-rt:hwasan Hardware-assisted address sanitizer compiler-rt:lsan Leak sanitizer compiler-rt:msan Memory sanitizer compiler-rt:sanitizer compiler-rt:tsan Thread sanitizer compiler-rt:ubsan Undefined behavior sanitizer compiler-rt flang:fir-hlfir flang:runtime flang:semantics flang Flang issues not falling into any other category libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc libunwind lld lldb llvm:analysis llvm:support llvm:transforms llvm-lit mlgo mlir:core MLIR Core Infrastructure mlir:linalg mlir openmp:libomp OpenMP host runtime openmp:libomptarget OpenMP offload runtime PGO Profile Guided Optimizations testing-tools tools:llvm-exegesis xray
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants