subject:"\[PATCH\] D132608\: \[CMake\] Clean up CMake binary dir handling"

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2023-04-12 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.
Herald added subscribers: bviyer, ekilmer, jplehr, thopre.

@Ericson2314  @phosek What's the state of this patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-11-07 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 added a comment.

I have done the deduping @phosek requested, and changed the variable names from 
`CMAKE_*` to `LLVMPROJ_*` which hopefully satisfies everyone's criteria. Happy 
with other non `LLVM_` options too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-11-07 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 473679.
Ericson2314 added a comment.
Herald added a subscriber: Moerafaat.

Dedup code, rename variables


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/LLVMGNUDirs.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,25 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +61,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,8 +7,18 @@
   project(openmp C CXX)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if (OPENMP_STANDALONE_BUILD)
   # CMAKE_BUILD_TYPE was not set, default to Release.
@@ -51,6 +54,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -7,14 +7,26 @@
 include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
   NO_POLICY_SCOPE)
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if(MLIR_STANDALONE_BUILD)
   find_package(LLVM CONFIG REQUIRED)
@@ -43,19 +55,16 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-16 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

I agree with @tianshilei1992, I think we should avoid introducing new `CMAKE_` 
variables to avoid confusion. The same applies to module names, for example I 
don't think we should be introducing `GNUBinaryDirs` which can be easily 
confused for `GNUInstallDirs`.

I would personally go with the `LLVM_` prefix, I agree that this can lead to 
some confusion given that LLVM is both the name of the top-level project as 
well as one of the subprojects, but I think that ship has already sailed we 
already have proliferation of `LLVM_` named variables throughout the codebase.

Regarding `GNUBinaryDirs`, it seems like we always use the following pattern:

  # Must go before the first `include(GNUInstallDirs)`.
  include(LLVMLibdirSuffix)
  
  # Must go below project(..)
  include(GNUInstallDirs)
  include(GNUBinaryDirs)

That introduces a lot of duplication and is potentially error-prone since we 
rely on particular include order. Could we instead introduce a single 
`LLVMInstallDirs` module which would contain the content of  `LLVMLibdirSuffix` 
and `GNUBinaryDirs` and would also include `GNUInstallDirs`, so 
`LLVMInstallDirs` is all you would need to include in individual subprojects?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 added a comment.

@tianshilei1992 that is a fair point. I would be open to calling them something 
else.

I just don't want to call them `LLVM_*` because that would be confusing since 
there is (a) LLVM in particular (b) the monorepo / project as a whole, and this 
variable are about *neither* of them, being scoped to the current build.

I mainly named this way to match the`GNUInstallDirs` naming scheme, where 
`CMAKE_INSTALL_PREFIX` goes with` CMAKE_INSTALL_*DIR`; here `CMAKE_BINARY_DIR` 
goes with `CMAKE_BINARY_*DIR`. I admit relative vs absolute paths, in addition 
to `_PREFIX` vs `_DIR`, do not make the analogy exact though.

I also was hoping something like this could be upstreamed into CMake itself, 
but using different names now doesn't prevent upstream from adopting `CMAKE_*` 
were this to become official.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread Shilei Tian via Phabricator via cfe-commits

tianshilei1992 added a comment.

Is it a good idea to define variables starting with `CMAKE`? That might be 
confusing for later maintenance by other developers because chances are that 
they will think those variables are CMake provided variables, try to look up 
into CMake document to see what they are, and then find out they are not at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 added inline comments.



Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

ldionne wrote:
> ldionne wrote:
> > Instead, I'd do this:
> > 
> > ```
> > if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
> >   set(_include_target_dir 
> > "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
> >   set(_install_library_dir 
> > "lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
> > else()
> >   set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
> >   set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
> > endif()
> > 
> > set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
> > "Path where target-specific libc++ headers should be installed.")
> > set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
> > "Path where built libc++ libraries should be installed.")
> > ```
> > 
> > IMO that's easier on the eye.
> Gentle ping on this.
Ah right, I have this "throw away temporary style as soon as possible" on all 
the runtimes right now. Would you prefer I do instead do "define all the 
defaults, define all the cache vars" for all of them?

I am a little partial to the current style because if it is closer to what I 
would do if cmake has proper expressions. (something like:
```
set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR
  "${
if(...)
  stuff
else()
 stuff
endif()
  }" CACHE PATH
"Path where target-specific libc++ headers should be 
```
). But if that if you are sure your want it consistently the other way I can 
switch it.

A bigger issue might be when the variables depend on each other, e.g. when I 
get the library install dir base name for the library binary dir.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added inline comments.



Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

ldionne wrote:
> Instead, I'd do this:
> 
> ```
> if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
>   set(_include_target_dir 
> "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
>   set(_install_library_dir 
> "lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
> else()
>   set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
>   set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
> endif()
> 
> set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
> "Path where target-specific libc++ headers should be installed.")
> set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
> "Path where built libc++ libraries should be installed.")
> ```
> 
> IMO that's easier on the eye.
Gentle ping on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 460191.
Ericson2314 added a comment.

Fix misspelled variable `CLANG_CURRENT_SOURCE_DIR` -> `CLANG_SOURCE_DIR`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/GNUBinaryDirs.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,28 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +64,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,6 +7,18 @@
   project(openmp C CXX)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
 
@@ -51,6 +56,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,13 +1,28 @@
 # MLIR project.
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(MLIR_STANDALONE_BUILD)
   cmake_minimum_required(VERSION 3.13.4)
@@ -36,12 +51,10 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
-set(MLIR_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
+set(MLIR_INCLUDE_DIR

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-14 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 460099.
Ericson2314 added a comment.

Fix typo in compiler-rt


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/GNUBinaryDirs.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,28 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +64,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,6 +7,18 @@
   project(openmp C CXX)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
 
@@ -51,6 +56,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,13 +1,28 @@
 # MLIR project.
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(MLIR_STANDALONE_BUILD)
   cmake_minimum_required(VERSION 3.13.4)
@@ -36,12 +51,10 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
-set(MLIR_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
+set(MLIR_INCLUDE_DIR ${MLIR_BINARY_DIR}/include)
 set(MLIR_TOOLS_DIR

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-13 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:3
+  get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME)
+endif()
+

compnerd wrote:
> Should this perhaps be moved further down near the usage?
Sure



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

compnerd wrote:
> arichardson wrote:
> > I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
> > (and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way 
> > it's clear that we are referring to binaries/libraries/includes in the 
> > build output (and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 
> I think you mean `${CMAKE_BINARY_DIR}/bin` as this is in a block where 
> `CMAKE_BINARY_BINDIR` is undefined.
@compnerd oops thanks.

@arichardson `CMAKE_BINARY_*` is the already existing naming convention of 
built-in variables. I agree it is not the best, but I don't want to buck the 
standard.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:10
+if (NOT DEFINED CMAKE_BINARY_INCLUDEDIR)
+  set(CMAKE_BINARY_INCLUDEDIR "${CMAKE_BINARY_DIR}/inc")
+endif()

compnerd wrote:
> Is the default spelling for include not `include` not `inc`?
Oops yes it is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-13 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 459985.
Ericson2314 marked 4 inline comments as done.
Ericson2314 added a comment.

Rebase, avoid sed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/GNUBinaryDirs.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,28 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +64,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,6 +7,18 @@
   project(openmp C CXX)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
 
@@ -51,6 +56,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,13 +1,28 @@
 # MLIR project.
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(MLIR_STANDALONE_BUILD)
   cmake_minimum_required(VERSION 3.13.4)
@@ -36,12 +51,10 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
-set(MLIR_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
+set(MLIR_INCLUDE_DIR

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-07 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added a comment.

I think I like the spirit of this change, which seems to be to move us closer 
to `CMAKE_foo` variables and further from `LLVM_foo` variables for equivalent 
functionality. I have a comment, but this essentially LGTM. The libc++ CI 
failures (in particular the bootstrapping build failure) seems to be real, 
though, so it should be understood before landing the patch.




Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

Instead, I'd do this:

```
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
  set(_include_target_dir 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
  set(_install_library_dir 
"lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
else()
  set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
  set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
endif()

set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
"Path where target-specific libc++ headers should be installed.")
set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
"Path where built libc++ libraries should be installed.")
```

IMO that's easier on the eye.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-07 Thread Saleem Abdulrasool via Phabricator via cfe-commits

compnerd added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:3
+  get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME)
+endif()
+

Should this perhaps be moved further down near the usage?



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

arichardson wrote:
> I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
> (and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way it's 
> clear that we are referring to binaries/libraries/includes in the build 
> output (and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 
I think you mean `${CMAKE_BINARY_DIR}/bin` as this is in a block where 
`CMAKE_BINARY_BINDIR` is undefined.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:10
+if (NOT DEFINED CMAKE_BINARY_INCLUDEDIR)
+  set(CMAKE_BINARY_INCLUDEDIR "${CMAKE_BINARY_DIR}/inc")
+endif()

Is the default spelling for include not `include` not `inc`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-08-25 Thread Sebastian Neubauer via Phabricator via cfe-commits

sebastian-ne added a comment.

I’m not sure if it’s the case for all places (as `CMAKE_CFG_INTDIR` is not 
defined at install time), but I think the `CMAKE_BINARY_LIBDIR` introduced here 
serves the same purpose as the already existing `LLVM_LIBRARY_DIR`.
Same for `CMAKE_BINARY_INCLUDEDIR`, which is `LLVM_INCLUDE_DIR` and 
`CMAKE_BINARY_BINDIR` which is `LLVM_TOOLS_BINARY_DIR`.

E.g. llvm/CMakeLists.txt uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )

while clang uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )

and this patch replaces the clang code with

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-08-25 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
(and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way it's 
clear that we are referring to binaries/libraries/includes in the build output 
(and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-08-24 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 created this revision.
Ericson2314 added reviewers: sebastian-ne, beanz, phosek, ldionne.
Herald added subscribers: libc-commits, libcxx-commits, Enna1, bzcheeseman, 
pmatos, asb, ayermolo, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, 
mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, pengfei, 
jgravelle-google, whisperity, sbc100, mgorny, nemanjai, dschuff.
Herald added a reviewer: bollu.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: NoQ.
Herald added projects: libunwind, libc-project, Flang, All.
Herald added a reviewer: libunwind.
Ericson2314 requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, yota9, sstefan1, stephenneuendorffer, 
nicolasvasilache, jdoerfert, aheejin.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

There are a few goals here:

- Match what D132316  for `LLVM_BINARY_DIR`, 
for `CMAKE_BINARY_DIR`.

- Decrease the usages of `LLVM_LIBDIR_SUFFIX`, preparing us for D130586 
.

- Deduplicate code repeated across projects

Do this in this way:

- Shuffle around `CMAKE_MODULE_PATH` appending to allow `include`-ing our own 
modules earlier.

- Create new private/internal CMake modules and use them:
  - `GNUBinaryDirs`

Like upstream CMake `GNUInstallDirs`, but sets variations of 
`CMAKE_BINARY_DIR`. These are not CACHE PATHS because they are not intended to 
be user-modifyable.

`CMAKE_BINARY_LIBDIR` is based on the base name of `CMAKE_INSTALL_LIBDIR` 
rather than `LLVM_LIBDIR_SUFFIX`. This cannot just end with "lib", because the 
"install" and "binary" base names need to line up for various relative paths to 
work. It doesn't just use `LLVM_LIBDIR_SUFFIX` because we are trying to phase 
that out.
  - `LLVMLibdirSuffix`

A compat shim that defaults `CMAKE_INSTALL_LIBDIR` based on 
`LLVM_LIBDIR_SUFFIX`.
  - `LLVMSetIntDirs`

Just here to deduplicate the defining of `LLVM_LIBRARY_OUTPUT_INTDIR` and 
friends between projects.

- Do these replacements to make use of deps
  - `${CMAKE_BINARY_DIR}/lib(${CMAKE_LIBDIR_SUFFIX})?\>` -> 
`${CMAKE_BINARY_LIBDIR}`
  - `${CMAKE_BINARY_DIR}/bin\>` -> `${CMAKE_BINARY_BINDIR}`
  - `${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX}` -> 
`${LLVM_LIBRARY_OUTPUT_INTDIR}`
  - `${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin` -> 
`${LLVM_RUNTIME_OUTPUT_INTDIR}`

It is somewhat odd to me that those last two vars start with `LLVM_` not 
`CMAKE_` when they are based on `CMAKE_BINARY_DIR`, but that can be tackled 
some other time.

- Cleanup custom install path initialization code in the runtimes

  Most significantly, they no longer have their


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  cmake/Modules/GNUBinaryDirs.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  flang/tools/f18/CMakeLists.txt
  libc/CMakeLists.txt
  libc/test/utils/tools/WrapperGen/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/bindings/python/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  llvm/tools/llvm-go/CMakeLists.txt
  llvm/unittests/Target/AArch64/CMakeLists.txt
  llvm/unittests/Target/PowerPC/CMakeLists.txt
  llvm/unittests/Target/WebAssembly/CMakeLists.txt
  llvm/unittests/Target/X86/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  mlir/test/CMakeLists.txt
  mlir/utils/mbr/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/remote/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,28 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

17 matches

Site Navigation

Mail list logo

Footer information