[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Thanks @stella.stamenova for looking into this. I agree the `extend_path` 
should go back so it properly becomes the install dir. I can't think why I 
removed it in the first place! The point was to not export the other var, and 
indeed having that one properly alternate between the build dir vs install dir 
is why we can get rid of exporting the other one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

In D117977#3680655 , 
@stella.stamenova wrote:

> This change broke the `LLVMConfig` generation and now when including `llvm` 
> through `LLVM_DIR` in another project such as `onnx-mlir`, various tools no 
> longer have the correct paths. For example, before this change:
>
>   set(LLVM_TOOLS_BINARY_DIR "${LLVM_INSTALL_PREFIX}/bin")
>   set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_INSTALL_PREFIX}/bin/llvm-lit")
>
> After this change:
>
>   set(LLVM_DEFAULT_EXTERNAL_LIT "/__w/1/b/llvm/Release/./bin/llvm-lit")
>   set(LLVM_TOOLS_BINARY_DIR "/__w/1/b/llvm/Release/./bin")
>
> Note that the new paths are both _absolute_ paths (which is not great for an 
> install scenario) as well as _wrong_ because they point to the build 
> directory even in an install scenario.
>
> The offending line is actually one that was removed from here: 
> `/llvm-project/llvm/cmake/modules/CMakeLists.txt`:
>
>   extend_path(LLVM_CONFIG_TOOLS_BINARY_DIR "\${LLVM_INSTALL_PREFIX}" 
> "${LLVM_TOOLS_INSTALL_DIR}")
>
> Previously (before this line was removed), `LLVM_CONFIG_TOOLS_BINARY_DIR ` 
> was set to the join of `LLVM_TOOLS_INSTALL_DIR` and `LLVM_INSTALL_PREFIX`, 
> but now that this is gone, it is simply set to:
>
>   set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
>
> where `LLVM_TOOLS_BINARY_DIR` is an absolute path to the binary directory 
> (which is fine for `LLVMConfig` in the binary tree) and 
> `LLVM_CONFIG_TOOLS_BINARY_DIR ` is never updated to point to an install 
> location for the install tree.

I am going to try https://reviews.llvm.org/D130545 which was committed today to 
see if it has resolved the issue. Linking here for reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

This change broke the `LLVMConfig` generation and now when including `llvm` 
through `LLVM_DIR` in another project such as `onnx-mlir`, various tools no 
longer have the correct paths. For example, before this change:

  set(LLVM_TOOLS_BINARY_DIR "${LLVM_INSTALL_PREFIX}/bin")
  set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_INSTALL_PREFIX}/bin/llvm-lit")

After this change:

  set(LLVM_DEFAULT_EXTERNAL_LIT "/__w/1/b/llvm/Release/./bin/llvm-lit")
  set(LLVM_TOOLS_BINARY_DIR "/__w/1/b/llvm/Release/./bin")

Note that the new paths are both _absolute_ paths (which is not great for an 
install scenario) as well as _wrong_ because they point to the build directory 
even in an install scenario.

The offending line is actually one that was removed from here: 
`/llvm-project/llvm/cmake/modules/CMakeLists.txt`:

  extend_path(LLVM_CONFIG_TOOLS_BINARY_DIR "\${LLVM_INSTALL_PREFIX}" 
"${LLVM_TOOLS_INSTALL_DIR}")

Previously (before this line was removed), `LLVM_CONFIG_TOOLS_BINARY_DIR ` was 
set to the join of `LLVM_TOOLS_INSTALL_DIR` and `LLVM_INSTALL_PREFIX`, but now 
that this is gone, it is simply set to:

  set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")

where `LLVM_TOOLS_BINARY_DIR` is an absolute path to the binary directory 
(which is fine for `LLVMConfig` in the binary tree) and 
`LLVM_CONFIG_TOOLS_BINARY_DIR ` is never updated to point to an install 
location for the install tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Will make patch forit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

@tstellar Thank you! That is an error I do understand, and yes your example is 
exactly right.

Sorry, I thought I had checked every rebase that no new usages cropped up, but 
I guess I missed one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

https://lab.llvm.org/buildbot#builders/74/builds/12262 here is a failure with 
only me in the blamelist, but I cannot see how those failures have anything to 
do with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

I think this commit broke our standalone builds:

CMake Error at utils/hmaptool/CMakeLists.txt:1 (install):

  install PROGRAMS given no DESTINATION!

Was the LLVM_TOOLS_INSTALL_DIR in this file supposed to be changed to 
CLANG_TOOLS_INSTALL_DIR ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Thanks @nikic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG07b749800c5c: [cmake] Dont export 
`LLVM_TOOLS_INSTALL_DIR` anymore (authored by Ericson2314).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ 

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Nikita Popov via Phabricator via cfe-commits]

nikic accepted this revision.
nikic added a comment.

Still LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

https://reviews.llvm.org/D130254 oh yay I learned from this the openmp failure 
is in fact spurious!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   MLIRPdllLspServerLib
   )
 
-add_llvm_tool(mlir-pdll-lsp-server
+add_mlir_tool(mlir-pdll-lsp-server
   mlir-pdll-lsp-server.cpp
 
   DEPENDS

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

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

Rebase again, to try to figure out if last round of errors were spurious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   MLIRPdllLspServerLib
   )
 
-add_llvm_tool(mlir-pdll-lsp-server

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Hmm, are all those errors from running out of file system space, or might there 
be another issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 marked an inline comment as done.
Ericson2314 added inline comments.



Comment at: openmp/tools/CMakeLists.txt:1-3
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)

Ericson2314 wrote:
> protze.joachim wrote:
> > Is this variable used anywhere?
> `llvm_add_tool` computes a usage.
Moved this stuff make sure it is used properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

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

Fix typo

Thank you @aaron.ballman and @nikic for finding the culprit!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   MLIRPdllLspServerLib
   )
 
-add_llvm_tool(mlir-pdll-lsp-server

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 reopened this revision.
Ericson2314 added a comment.
This revision is now accepted and ready to land.

Thanks for finding this and sorry for the disruption. Hope the revert didn't 
come to slowly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:178
   # Always generate install targets
-  llvm_install_symlink(${name} ${dest} ALWAYS_GENERATE)
+  llvm_install_symlink(CLANG name} ${dest} ALWAYS_GENERATE)
 endmacro()

Yeah, there's a typo on this line. Should be `${name}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

We saw the same thing at Intel FWIW. When I investigated it, all of the 
symlinks were being created as `name}` instead of whatever you'd expect the 
symlink to be named.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

Just another data point, but we noticed the same issue of clang++ missing after 
install on our internal Windows builds, and I bisected it to this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

There might be something fishy with this commit.  My fresh from-scratch `cmake 
--build . && cmake --build . --target install` monorepo build now lacked e.g. 
the `bin/clang++` symlink in the installation dir, and locally reverting this 
commit fixed that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd5daa5c5b091: [cmake] Dont export 
`LLVM_TOOLS_INSTALL_DIR` anymore (authored by Ericson2314).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ 

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Nikita Popov via Phabricator via cfe-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added inline comments.



Comment at: bolt/tools/CMakeLists.txt:11-13
+macro(add_bolt_tool_symlink name)
+  llvm_add_tool_symlink(BOLT ${ARGV})
+endmacro()

mizvekov wrote:
> Instead of each project having to define a different macro for this, why not 
> just do this based on the current project in scope?
> I think this would be a less invasive change.
> 
> FWIW my review can only help in these small matters of coding, I can't really 
> review this from the perspective of the intended effect, I am just a clang 
> developer and I never have to package or install LLVM I built myself, I just 
> test stuff from the build dir :)
`project` is not redefined in the unified / monorepo build, so we cannot rely 
on it.



Comment at: bolt/tools/CMakeLists.txt:11-13
+macro(add_bolt_tool_symlink name)
+  llvm_add_tool_symlink(BOLT ${ARGV})
+endmacro()

Ericson2314 wrote:
> mizvekov wrote:
> > Instead of each project having to define a different macro for this, why 
> > not just do this based on the current project in scope?
> > I think this would be a less invasive change.
> > 
> > FWIW my review can only help in these small matters of coding, I can't 
> > really review this from the perspective of the intended effect, I am just a 
> > clang developer and I never have to package or install LLVM I built myself, 
> > I just test stuff from the build dir :)
> `project` is not redefined in the unified / monorepo build, so we cannot rely 
> on it.
> I am just a clang developer and I never have to package or install LLVM I 
> built myself, I just test stuff from the build dir :)

You mean you build clang against a prebuilt LLVM? I am confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.
Herald added a subscriber: Peiming.



Comment at: bolt/tools/CMakeLists.txt:11-13
+macro(add_bolt_tool_symlink name)
+  llvm_add_tool_symlink(BOLT ${ARGV})
+endmacro()

Instead of each project having to define a different macro for this, why not 
just do this based on the current project in scope?
I think this would be a less invasive change.

FWIW my review can only help in these small matters of coding, I can't really 
review this from the perspective of the intended effect, I am just a clang 
developer and I never have to package or install LLVM I built myself, I just 
test stuff from the build dir :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added subscribers: mizvekov, nikic.
Ericson2314 added a comment.

@nikic @mizvekov inspired by the recent activity in D126308 
, I rebased this. Would you mind giving it a 
review or trying out a local build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   MLIRPdllLspServerLib
   )
 
-add_llvm_tool(mlir-pdll-lsp-server
+add_mlir_tool(mlir-pdll-lsp-server
   mlir-pdll-lsp-server.cpp
 
   DEPENDS

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.
Herald added a subscriber: bzcheeseman.
Herald added a project: All.

I moved the `openmp` definitions so they should be used. Don't need the other 
variables yet I don't think?




Comment at: openmp/tools/CMakeLists.txt:1-3
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)

protze.joachim wrote:
> Is this variable used anywhere?
`llvm_add_tool` computes a usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

I think in openmp/tools currently only libraries and header files get 
installed, but no binaries.

I suggest to define `OPENMP_TOOLS_INSTALL_BINDIR`, 
`OPENMP_TOOLS_INSTALL_LIBDIR`, and `OPENMP_TOOLS_INSTALL_INCLUDEDIR` and 
probably base these variables on OPENMP_TOOLS_INSTALL_DIR, if defined (e.g., 
for distro builds), or OPENMP_INSTALL_{BIN|LIB|INCLUDE}DIR as fallback.

openmp/tools/*/CMakeLists.txt should then use these variables instead of the 
various variables they use currently.




Comment at: openmp/tools/CMakeLists.txt:1-3
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)

Is this variable used anywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Could one of you review this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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