[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D79219#2153585 , @phosek wrote:

> In D79219#2152747 , @labath wrote:
>
>> I wouldn't mind separate (internal) cache variable, though I am somewhat 
>> surprised by this problem. A (non-cache) variable set in one of the parent 
>> scopes should still take precedence over a cache variable with the same 
>> name. And since config-ix.cmake is included from the top CMakeLists.txt, the 
>> value it defines should be available everywhere. Was this a problem for the 
>> regular build, or only for some of the more exotic build setups that don't 
>> start with llvm/CMakeLists.txt ?
>
> Never mind, it's working as expected. The problem is that we disable zlib 
> detection on Windows which I missed before. I'm not sure why that's the case, 
> I tested this on Windows and it seems to be working fine, but for now I've 
> kept the existing behavior, we can consider enabling zlib on Windows in a 
> follow up change.

:-( We rely on zlib on Windows, and we pass -DLLVM_ENABLE_ZLIB=FORCE_ON to 
ensure that the build breaks if it can't be used for some reason.

It seems your change both disabled use of zlib on Windows, and also removed the 
check which made that an error when using FORCE_ON, which means we didn't catch 
this until further down the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Reverted in 3ab01550b632dad46f9595d74855749557ffd25c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-06 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 283398.
phosek marked an inline comment as done.

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -28,6 +28,7 @@
 
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
+
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitiz

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-06 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1adc494bce44: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D79219?vs=283398&id=283434#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-06 Thread Petr Hosek via Phabricator via cfe-commits
phosek reopened this revision.
phosek added a comment.
This revision is now accepted and ready to land.

Sorry about the breakage, that was an unintentional change. I have updated the 
patch and restored the original behavior on Windows.


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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-06 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: llvm/cmake/config-ix.cmake:178
 
-if (LLVM_ENABLE_ZLIB STREQUAL "FORCE_ON" AND NOT HAVE_LIBZ)
-  message(FATAL_ERROR "Failed to configure zlib")

Could this check be put back? E.g. for now it seems building with 
-DLLVM_USE_SANITIZERS=Memory and -DLLVM_ENABLE_ZLIB=FORCE_ON will silently not 
use zlib.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

@phosek, Under this change, now when I build LLVM (with a basic config `cmake 
-G Ninja --LLVM_ENABLE_PROJECTS="clang" ../llvm`), in file 
`build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see this:

  set_target_properties(LLVMSupport PROPERTIES
INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"

This seems broken to me. Can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

How can I test this change on pre-merge bots? I haven't seen any builds posted 
on this change before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

In D79219#2202048 , @phosek wrote:

> How can I test this change on pre-merge bots? I haven't seen any builds 
> posted on this change before.

You can just look at the mlir-windows buildbot, this also showed the failing 
build: http://lab.llvm.org:8011/builders/mlir-windows?numbuilds=200
However it's lagging behind quite a bit.

Pre-merge testing is using a different infrastructure and is not sending 
notifications on failing master build. We're still trying to figure out how to 
deal with a broken master branch properly without building every revision...
Right now it's building master every 4 hours using the same setup used for 
pre-merge testing: https://buildkite.com/llvm-project/llvm-master-build/builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

This patch broke the Windows compilation on buildbot 
 and pre-merge 
testing . So 
I'll revert it to get pre-merge testing back online. Otherwise this will cause 
false-positives for pre-merge testing.

The error message:

  FAILED: bin/llvm-profdata.exe 
  cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
--intdir=tools\llvm-profdata\CMakeFiles\llvm-profdata.dir --rc="C:\Program 
Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\rc.exe" --mt="C:\Program Files 
(x86)\Windows Kits\10\bin\10.0.18362.0\x64\mt.exe" --manifests  -- 
C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
  /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
/pdb:bin\llvm-profdata.pdb /version:0.0  /machine:x64 /STACK:1000 
/INCREMENTAL:NO /subsystem:console  lib\LLVMCore.lib lib\LLVMProfileData.lib 
lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib delayimp.lib 
-delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib 
user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib 
comdlg32.lib advapi32.lib && cd ."
  LINK: command 
"C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
 /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
/pdb:bin\llvm-profdata.pdb /version:0.0 /machine:x64 /STACK:1000 
/INCREMENTAL:NO /subsystem:console lib\LLVMCore.lib lib\LLVMProfileData.lib 
lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib delayimp.lib 
-delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib 
user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib 
comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTFILE:bin\llvm-profdata.exe.manifest" failed (exit code 1120) with the 
following output:
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol compress2 referenced in function "class llvm::Error __cdecl 
llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,int)" 
(?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol compressBound referenced in function "class llvm::Error __cdecl 
llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,int)" 
(?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol uncompress referenced in function "class llvm::Error __cdecl 
llvm::zlib::uncompress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,unsigned __int64)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
Hint on symbols that are defined and could potentially match:
  "class llvm::Error __cdecl llvm::zlib::uncompress(class 
llvm::StringRef,class llvm::SmallVectorImpl &,unsigned __int64)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
  "class llvm::Error __cdecl llvm::zlib::uncompress(class 
llvm::StringRef,char *,unsigned __int64 &)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@PEADAEA_K@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol crc32 referenced in function "unsigned int __cdecl 
llvm::zlib::crc32(class llvm::StringRef)" (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
Hint on symbols that are defined and could potentially match:
  "unsigned int __cdecl llvm::zlib::crc32(class llvm::StringRef)" 
(?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
  C:\GnuWin\lib\zlib.lib : warning LNK4272: library machine type 'x86' 
conflicts with target machine type 'x64'
  bin\llvm-profdata.exe : fatal error LNK1120: 4 unresolved externals


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2201109 , @lxfind wrote:

> @phosek, Under this change, now when I build LLVM (with a basic config `cmake 
> -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm`), in file 
> `build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see this:
>
>   set_target_properties(LLVMSupport PROPERTIES
> INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"
>
> This seems broken to me. Can you take a look?

This is correct. That target is provided by `find_package(ZLIB)`. In 
LLVMConfig.cmake, we invoke `find_package(ZLIB)` when zlib support is enabled. 
If you're using `LLVMExports.cmake`, you'll need to invoke `find_package(ZLIB)` 
yourself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2201994 , @kuhnel wrote:

> This patch broke the Windows compilation on buildbot 
>  and pre-merge 
> testing . So 
> I'll revert it to get pre-merge testing back online. Otherwise this will 
> cause false-positives for pre-merge testing.
>
> The error message:
>
>   FAILED: bin/llvm-profdata.exe 
>   cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
> --intdir=tools\llvm-profdata\CMakeFiles\llvm-profdata.dir --rc="C:\Program 
> Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\rc.exe" --mt="C:\Program 
> Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\mt.exe" --manifests  -- 
> C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
>   /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
> /pdb:bin\llvm-profdata.pdb /version:0.0  /machine:x64 /STACK:1000 
> /INCREMENTAL:NO /subsystem:console  lib\LLVMCore.lib lib\LLVMProfileData.lib 
> lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
> lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
> shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib 
> delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib 
> kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
> oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
>   LINK: command 
> "C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
>  /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
> /pdb:bin\llvm-profdata.pdb /version:0.0 /machine:x64 /STACK:1000 
> /INCREMENTAL:NO /subsystem:console lib\LLVMCore.lib lib\LLVMProfileData.lib 
> lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
> lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
> shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib 
> delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib 
> kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
> oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
> /MANIFESTFILE:bin\llvm-profdata.exe.manifest" failed (exit code 1120) with 
> the following output:
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol compress2 referenced in function "class llvm::Error __cdecl 
> llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
> &,int)" 
> (?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol compressBound referenced in function "class llvm::Error __cdecl 
> llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
> &,int)" 
> (?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol uncompress referenced in function "class llvm::Error __cdecl 
> llvm::zlib::uncompress(class llvm::StringRef,class 
> llvm::SmallVectorImpl &,unsigned __int64)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
> Hint on symbols that are defined and could potentially match:
>   "class llvm::Error __cdecl llvm::zlib::uncompress(class 
> llvm::StringRef,class llvm::SmallVectorImpl &,unsigned __int64)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
>   "class llvm::Error __cdecl llvm::zlib::uncompress(class 
> llvm::StringRef,char *,unsigned __int64 &)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@PEADAEA_K@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol crc32 referenced in function "unsigned int __cdecl 
> llvm::zlib::crc32(class llvm::StringRef)" 
> (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
> Hint on symbols that are defined and could potentially match:
>   "unsigned int __cdecl llvm::zlib::crc32(class llvm::StringRef)" 
> (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
>   C:\GnuWin\lib\zlib.lib : warning LNK4272: library machine type 'x86' 
> conflicts with target machine type 'x64'
>   bin\llvm-profdata.exe : fatal error LNK1120: 4 unresolved externals

Looking at the error, it seems like you have a 32-bit version of zlib installed 
and in your search path on a 64-bit version of Windows. I'm not quite sure how 
to handle that in CMake.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  h

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

> Looking at the error, it seems like you have a 32-bit version of zlib 
> installed and in your search path on a 64-bit version of Windows. I'm not 
> quite sure how to handle that in CMake.

For mlir-windows buildbot:
I have no clue on what is installed there.

For pre-merge testing:
I guess zlib is part of GNUWin32 package being installed. This is the 
recommended setup for Visual Studio 
. The pre-merge builds are running 
in a docker container on Windows 
,
 so you can check what software is being installed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-08 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 284106.

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-08 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

In D79219#2201415 , @phosek wrote:

> In D79219#2201109 , @lxfind wrote:
>
>> @phosek, Under this change, now when I build LLVM (with a basic config 
>> `cmake -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm`), in file 
>> `build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see this:
>>
>>   set_target_properties(LLVMSupport PROPERTIES
>> INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"
>>
>> This seems broken to me. Can you take a look?
>
> This is correct. That target is provided by `find_package(ZLIB)`. In 
> LLVMConfig.cmake, we invoke `find_package(ZLIB)` when zlib support is 
> enabled. If you're using `LLVMExports.cmake`, you'll need to invoke 
> `find_package(ZLIB)` yourself.

@phosek, Thanks for the reply. I am not too familiar with this. Could you 
please elaborate more on why setting it to `ZLIB::ZLIB` here is more modern? 
And how one is expected to deal with `ZLIB::ZLIB` that shows up in 
LLVMExports.cmake` (in the modern way)? Should one string pattern matching for 
`ZLIB::ZLIB` and invoke `find_package(ZLIB)` again? (previously one could 
simply concatenate `-l` with the library name in there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

I have finally managed to reproduce the issue on my Windows machine, the latest 
version of the patch should address the issue. The previous logic would fail to 
find zlib altogether on Windows, which is why this issue haven't manifested 
before. `find_package` locates the zlib correctly, but we need to ensure that 
the library that was found can be used.


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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 284161.

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-09 Thread Petr Hosek via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGccbc1485b55f: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-10 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

@kuhnel do you want to take a look or is it okay if I just submit the change?


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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-12 Thread Petr Hosek via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG31e5f7120bdd: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-12 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2203887 , @lxfind wrote:

> In D79219#2201415 , @phosek wrote:
>
>> This is correct. That target is provided by `find_package(ZLIB)`. In 
>> LLVMConfig.cmake, we invoke `find_package(ZLIB)` when zlib support is 
>> enabled. If you're using `LLVMExports.cmake`, you'll need to invoke 
>> `find_package(ZLIB)` yourself.
>
> @phosek, Thanks for the reply. I am not too familiar with this. Could you 
> please elaborate more on why setting it to `ZLIB::ZLIB` here is more modern? 
> And how one is expected to deal with `ZLIB::ZLIB` that shows up in 
> LLVMExports.cmake` (in the modern way)? Should one string pattern matching 
> for `ZLIB::ZLIB` and invoke `find_package(ZLIB)` again? (previously one could 
> simply concatenate `-l` with the library name in there.

@lxfind `ZLIB::ZLIB` is the name of the imported target that's defined by CMake 
when you invoke `find_dependency(ZLIB)`. Ideally, you'd just use 
`LLVMConfig.cmake` which invokes that for you if needed. 
https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/ has some 
more details.

`ZLIB::ZLIB` includes the correct include and library directory, so you no 
longer have to manually append `-I` or `-l`, all you need is to include 
`ZLIB::ZLIB` in the list of your dependencies, which is one advantage. Another 
advantage is that `ZLIB::ZLIB` can expand to different things, for example some 
users may want to statically link zlib in which case they would configure their 
build accordingly and `ZLIB::ZLIB` would expand into `/path/to/libz.a` (this is 
for example what we do in our toolchain build), others may be fine using the 
default system library in which case `ZLIB::ZLIB` would typically expand to 
just `z`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-12 Thread Harmen Stoppels via Phabricator via cfe-commits
haampie added a comment.

Great, one benefit of this is that zlib can now be detected in non-system libs. 
Maybe we should handle ncurses / TERMINFO in a similar manner? It currently has 
similar logic as finding zlib had


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-12 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

@phosek sorry for the late reply, the builds on master are still green, so your 
changes are working on Windows as well.

Thank you for the extra work!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-06-29 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 274256.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-06-29 Thread Petr Hosek via Phabricator via cfe-commits
phosek marked an inline comment as done.
phosek added inline comments.



Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

labath wrote:
> JDevlieghere wrote:
> > phosek wrote:
> > > JDevlieghere wrote:
> > > > If I understand correctly, after running the configuration phase with 
> > > > LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by 
> > > > ON? 
> > > Correct, we used `FORCE_ON` above when invoking `find_package` setting 
> > > `REQUIRED` which makes the check fail if the library is missing. 
> > > Recording this information is important for LLVM consumers because it'll 
> > > be stored in `LLVMConfig.cmake` and AFAIU we don't want to propagate 
> > > `FORCE_ON` there.
> > I get that. My worry is that if for whatever reason the library disappears 
> > (system upgrade, package removal, ...) this will silently disable ZLIB 
> > support because now LLVM_ENABLE_ZLIB just equals on. This might sound far 
> > fetched, but happens all the time with "internal installs". This is why I 
> > prefer the ON/OFF/Auto approach because it doesn't update the cache 
> > variable, but would set the internal variable to either ON or OFF.
> I agree with Jonas, though I don't think the actual values (FORCE_ON/ON vs. 
> ON/Auto) really matter here. The important part is not overwriting the cache 
> variables specified by the user, as that can create various problems with 
> reconfigurations (like the zlib becoming unavailable example).
I've updated the change to shadow the variable as suggested by Jonas.


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-06-29 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 274255.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-06-30 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks Petr, LGTM


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-04-30 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: smeenai, compnerd.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, 
hiraditya, mgorny.
Herald added projects: clang, Sanitizers, LLDB, LLVM.

Rather than handling zlib handling manually, use `find_package` from CMake
to find zlib properly. Use this to normalize the `LLVM_ENABLE_ZLIB`,
`HAVE_ZLIB`, `HAVE_ZLIB_H`. Furthermore, require zlib if `LLVM_ENABLE_ZLIB` is
set to `YES`, which requires the distributor to explicitly select whether
zlib is enabled or not. This simplifies the CMake handling and usage in
the rest of the tooling.

This is a reland of rGabb0075 
 with all 
followup changes and fixes that should address
issues that were reported in PR44780.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -192,10 +192,31 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_libra

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-08 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments.



Comment at: lld/test/lit.site.cfg.py.in:16
 config.target_triple = "@TARGET_TRIPLE@"
-config.python_executable = "@Python3_EXECUTABLE@"
-config.have_zlib = @HAVE_LIBZ@
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.have_zlib = @LLVM_ENABLE_ZLIB@

This seems like an unintended/unrelated change.



Comment at: llvm/cmake/config-ix.cmake:506
 
-if (LLVM_ENABLE_ZLIB )
-  # Check if zlib is available in the system.
-  if ( NOT HAVE_ZLIB_H OR NOT HAVE_LIBZ )
-set(LLVM_ENABLE_ZLIB 0)
+if(LLVM_ENABLE_ZLIB)
+  if(LLVM_ENABLE_ZLIB STREQUAL FORCE_ON)

How come you're moving this out of the MSan check block? Isn't that a behavior 
change?



Comment at: llvm/lib/Support/CMakeLists.txt:207
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND
+ zlib_library MATCHES 
"^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")

Even if there's no prefix, wouldn't you still wanna strip the suffix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-11 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added a comment.

I'm in favor of this change. I'm not too happy with how this works in CMake, 
I've expressed similar concerns when the FORCE_ON approach was suggested in 
D71306 .

I really like what we ended up with in LLDB. The TL;DR is that we have 3 
values: ON, OFF and Auto. The later will resolve to either ON or OFF depending 
on whether the dependency was found. The "clever" part is that we use shadowing 
to avoid having another CMake variable without overwriting the cache.

Here's what it looks like in LLDB: 
https://github.com/llvm/llvm-project/blob/3df40007e6317955176dff0fa9e0b029dc4a11d1/lldb/cmake/modules/LLDBConfig.cmake#L26

Maybe we can consider something similar here? I'd be more than happy to hoist 
the corresponding CMake into llvm.

(edit: added myself as a reviewer so this shows up in my review queue)




Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

If I understand correctly, after running the configuration phase with 
LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by ON? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-11 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 263288.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -192,10 +192,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-11 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 263284.
phosek marked 2 inline comments as done.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -192,10 +192,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zli

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-11 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2029520 , @JDevlieghere 
wrote:

> I'm in favor of this change. I'm not too happy with how this works in CMake, 
> I've expressed similar concerns when the FORCE_ON approach was suggested in 
> D71306 .
>
> I really like what we ended up with in LLDB. The TL;DR is that we have 3 
> values: ON, OFF and Auto. The later will resolve to either ON or OFF 
> depending on whether the dependency was found. The "clever" part is that we 
> use shadowing to avoid having another CMake variable without overwriting the 
> cache.
>
> Here's what it looks like in LLDB: 
> https://github.com/llvm/llvm-project/blob/3df40007e6317955176dff0fa9e0b029dc4a11d1/lldb/cmake/modules/LLDBConfig.cmake#L26
>
> Maybe we can consider something similar here? I'd be more than happy to hoist 
> the corresponding CMake into llvm.
>
> (edit: added myself as a reviewer so this shows up in my review queue)


I'm fine either way, to me ON/OFF/AUTO and FORCE_ON/ON/OFF is just a different 
way to spell the same thing, my only concern would be breaking existing users 
who might be already relying on FORCE_ON.




Comment at: llvm/cmake/config-ix.cmake:506
 
-if (LLVM_ENABLE_ZLIB )
-  # Check if zlib is available in the system.
-  if ( NOT HAVE_ZLIB_H OR NOT HAVE_LIBZ )
-set(LLVM_ENABLE_ZLIB 0)
+if(LLVM_ENABLE_ZLIB)
+  if(LLVM_ENABLE_ZLIB STREQUAL FORCE_ON)

smeenai wrote:
> How come you're moving this out of the MSan check block? Isn't that a 
> behavior change?
This was done in the original change, I'm not quite sure what's the motivation, 
@compnerd do you know?



Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

JDevlieghere wrote:
> If I understand correctly, after running the configuration phase with 
> LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by ON? 
Correct, we used `FORCE_ON` above when invoking `find_package` setting 
`REQUIRED` which makes the check fail if the library is missing. Recording this 
information is important for LLVM consumers because it'll be stored in 
`LLVMConfig.cmake` and AFAIU we don't want to propagate `FORCE_ON` there.


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-11 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added inline comments.



Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

phosek wrote:
> JDevlieghere wrote:
> > If I understand correctly, after running the configuration phase with 
> > LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by ON? 
> Correct, we used `FORCE_ON` above when invoking `find_package` setting 
> `REQUIRED` which makes the check fail if the library is missing. Recording 
> this information is important for LLVM consumers because it'll be stored in 
> `LLVMConfig.cmake` and AFAIU we don't want to propagate `FORCE_ON` there.
I get that. My worry is that if for whatever reason the library disappears 
(system upgrade, package removal, ...) this will silently disable ZLIB support 
because now LLVM_ENABLE_ZLIB just equals on. This might sound far fetched, but 
happens all the time with "internal installs". This is why I prefer the 
ON/OFF/Auto approach because it doesn't update the cache variable, but would 
set the internal variable to either ON or OFF.


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-12 Thread Pavel Labath via Phabricator via cfe-commits
labath added inline comments.



Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

JDevlieghere wrote:
> phosek wrote:
> > JDevlieghere wrote:
> > > If I understand correctly, after running the configuration phase with 
> > > LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by ON? 
> > Correct, we used `FORCE_ON` above when invoking `find_package` setting 
> > `REQUIRED` which makes the check fail if the library is missing. Recording 
> > this information is important for LLVM consumers because it'll be stored in 
> > `LLVMConfig.cmake` and AFAIU we don't want to propagate `FORCE_ON` there.
> I get that. My worry is that if for whatever reason the library disappears 
> (system upgrade, package removal, ...) this will silently disable ZLIB 
> support because now LLVM_ENABLE_ZLIB just equals on. This might sound far 
> fetched, but happens all the time with "internal installs". This is why I 
> prefer the ON/OFF/Auto approach because it doesn't update the cache variable, 
> but would set the internal variable to either ON or OFF.
I agree with Jonas, though I don't think the actual values (FORCE_ON/ON vs. 
ON/Auto) really matter here. The important part is not overwriting the cache 
variables specified by the user, as that can create various problems with 
reconfigurations (like the zlib becoming unavailable example).


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-05-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai requested changes to this revision.
smeenai added a comment.
This revision now requires changes to proceed.

Requesting changes to remove from my review queue while the above comments are 
being addressed.


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

Oops, missed that I'd become a blocking reviewer for this (cos of requesting 
changes before).


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-14 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c1a79dc12f3: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND 

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-14 Thread Petr Hosek via Phabricator via cfe-commits
phosek reopened this revision.
phosek added a comment.
This revision is now accepted and ready to land.

I had to revert this change because it broke bots that don't have zlib 
installed. What I haven't realized is that the shadowed variable will only be 
accessible from the same file. I could move the logic from 
`llvm/lib/Support/CMakeLists.txt` into `llvm/cmake/config-ix.cmake`, but that 
doesn't handle all the other files that need to access that variable, such as 
all the `*/test/CMakeLists.txt` files. So I don't know if shadowing is the way 
to go here. Maybe we should set a different internal cached variable 
(`HAVE_LIBZ`?) and use that to check if zlib is available. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-15 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment.

I wouldn't mind separate (internal) cache variable, though I am somewhat 
surprised by this problem. A (non-cache) variable set in one of the parent 
scopes should still take precedence over a cache variable with the same name. 
And since config-ix.cmake is included from the top CMakeLists.txt, the value it 
defines should be available everywhere. Was this a problem for the regular 
build, or only for some of the more exotic build setups that don't start with 
llvm/CMakeLists.txt ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-15 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 278216.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-15 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2152747 , @labath wrote:

> I wouldn't mind separate (internal) cache variable, though I am somewhat 
> surprised by this problem. A (non-cache) variable set in one of the parent 
> scopes should still take precedence over a cache variable with the same name. 
> And since config-ix.cmake is included from the top CMakeLists.txt, the value 
> it defines should be available everywhere. Was this a problem for the regular 
> build, or only for some of the more exotic build setups that don't start with 
> llvm/CMakeLists.txt ?


Never mind, it's working as expected. The problem is that we disable zlib 
detection on Windows which I missed before. I'm not sure why that's the case, I 
tested this on Windows and it seems to be working fine, but for now I've kept 
the existing behavior, we can consider enabling zlib on Windows in a follow up 
change.


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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-23 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d09ecf36175: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND 

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-23 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 280305.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,32 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND CMAKE_STATIC_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND CMAKE_SHARED_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*${CMAKE_SHARED_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_librar

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-24 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG10b1b4a231a4: [CMake] Simplify CMake handling for zlib 
(authored by phosek).
Herald added subscribers: msifontes, jurahul, Kayjukh, grosul1, Joonsoo, 
stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, 
nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini.
Herald added a project: MLIR.

Changed prior to commit:
  https://reviews.llvm.org/D79219?vs=280305&id=280331#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -28,6 +28,11 @@
 
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
+
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
+
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,32 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-25 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment.

@phosek I suspect this is causing a cmake error on the lldb standalone bot, 
would you mind taking a look?

http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake-standalone/1858/

  CMake Error at 
/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/lib/cmake/llvm/AddLLVM.cmake:823
 (add_executable):
Target "lit-cpuid" links to target "ZLIB::ZLIB" but the target was not
found.  Perhaps a find_package() call is missing for an IMPORTED target, or
an ALIAS target is missing?
  Call Stack (most recent call first):
cmake/modules/AddLLDB.cmake:165 (add_llvm_executable)
utils/lit-cpuid/CMakeLists.txt:1 (add_lldb_executable)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-26 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

mehdi_amini wrote:
> I am a bit unsure that it is desirable that it is desirable to need this 
> change?
> This requires every single user of LLVM to do this. Is there a way this can 
> be done in the call to `find_package(LLVM)` instead?
(Doc for the usual way to integrate LLVM: 
https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-26 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

I am a bit unsure that it is desirable that it is desirable to need this change?
This requires every single user of LLVM to do this. Is there a way this can be 
done in the call to `find_package(LLVM)` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-26 Thread Stephen Neuendorffer via Phabricator via cfe-commits
stephenneuendorffer added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

mehdi_amini wrote:
> mehdi_amini wrote:
> > I am a bit unsure that it is desirable that it is desirable to need this 
> > change?
> > This requires every single user of LLVM to do this. Is there a way this can 
> > be done in the call to `find_package(LLVM)` instead?
> (Doc for the usual way to integrate LLVM: 
> https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )
I tend to agree with Mehdi.  I think stuff like this which is *required* to use 
LLVM should get pushed into what LLVM exports.  (Actually there is a bunch of 
other configuration stuff which also needs to get pushed into the export, 
otherwise configuration defaults are all wrong and things like that.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-27 Thread Petr Hosek via Phabricator via cfe-commits
phosek marked 4 inline comments as done.
phosek added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

stephenneuendorffer wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > I am a bit unsure that it is desirable that it is desirable to need this 
> > > change?
> > > This requires every single user of LLVM to do this. Is there a way this 
> > > can be done in the call to `find_package(LLVM)` instead?
> > (Doc for the usual way to integrate LLVM: 
> > https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )
> I tend to agree with Mehdi.  I think stuff like this which is *required* to 
> use LLVM should get pushed into what LLVM exports.  (Actually there is a 
> bunch of other configuration stuff which also needs to get pushed into the 
> export, otherwise configuration defaults are all wrong and things like that.)
Implemented in D84691.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-19 Thread Harmen Stoppels via Phabricator via cfe-commits
haampie added inline comments.



Comment at: llvm/lib/Support/CMakeLists.txt:217
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND CMAKE_SHARED_LIBRARY_SUFFIX AND
+  zlib_library MATCHES 
"^${CMAKE_SHARED_LIBRARY_PREFIX}.*${CMAKE_SHARED_LIBRARY_SUFFIX}$")

This will not work on macOS since cmake will find stub libraries by default 
(/path/to/libz.tbd), see 
https://github.com/Kitware/CMake/blob/master/Modules/Platform/Darwin.cmake#L71 
for the search order.

Instead you most likely want to use `CMAKE_FIND_LIBRARY_PREFIXES` and 
`CMAKE_FIND_LIBRARY_SUFFIXES`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-22 Thread Mateusz Mikuła via Phabricator via cfe-commits
mati865 added a comment.

@phosek in MSYS2 (targeting x86_64-w64-windows-gnu) Zlib works properly for 
LLVM 10 but with master I'm now seeing:

  -- Constructing LLVMBuild project information
  -- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a
  CMake Error at lib/Support/CMakeLists.txt:9 (string):
string sub-command REGEX, mode REPLACE: regex "^(lib|)" matched an empty
string.
  Call Stack (most recent call first):
lib/Support/CMakeLists.txt:226 (get_system_libname)

`-- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a` was printed by my 
change to help debugging it.
FYI `zlib_library` is set here: 
https://github.com/llvm/llvm-project/blob/8e06bf6b3a2e8d25e56cd52dca0cf3ff1b37b5d1/llvm/lib/Support/CMakeLists.txt#L218


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-23 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D79219#2231927 , @mati865 wrote:

> @phosek in MSYS2 (targeting x86_64-w64-windows-gnu) Zlib works properly for 
> LLVM 10 but with master I'm now seeing:
>
>   -- Constructing LLVMBuild project information
>   -- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a
>   CMake Error at lib/Support/CMakeLists.txt:9 (string):
> string sub-command REGEX, mode REPLACE: regex "^(lib|)" matched an empty
> string.
>   Call Stack (most recent call first):
> lib/Support/CMakeLists.txt:226 (get_system_libname)
>
> `-- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a` was printed by my 
> change to help debugging it.
> FYI `zlib_library` is set here: 
> https://github.com/llvm/llvm-project/blob/8e06bf6b3a2e8d25e56cd52dca0cf3ff1b37b5d1/llvm/lib/Support/CMakeLists.txt#L218

Looks like that's an issue introduced by D86134 
 or D86245 . 
Can you print the value of `${CMAKE_FIND_LIBRARY_PREFIXES}` and 
`${CMAKE_FIND_LIBRARY_SUFFIXES}` in your build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-23 Thread Mateusz Mikuła via Phabricator via cfe-commits
mati865 added a comment.

@phosek

> Looks like that's an issue introduced by D86134 
>  or D86245 .

Indeed, I apologize for bothering you. Should I move the discussion to one of 
patches created by  @haampie?

Continuing the discussion with `if( CMAKE_FIND_LIBRARY_PREFIXES )` and `if( 
CMAKE_FIND_LIBRARY_SUFFIXES )` from https://reviews.llvm.org/D86245 changed to 
false `llvm-config --system-libs` prints `-llibz.dll.a` so we need those 
replaces to bring back correct `-lz` value.

> Can you print the value of ${CMAKE_FIND_LIBRARY_PREFIXES} and 
> ${CMAKE_FIND_LIBRARY_SUFFIXES} in your build?

  -- CMAKE_FIND_LIBRARY_PREFIXES=lib;
  -- CMAKE_FIND_LIBRARY_SUFFIXES=.dll.a;.a;.lib

Which matches 
https://gitlab.kitware.com/cmake/cmake/-/blob/269d1a86249ea037a2884133daffc8c44a38d926/Modules/Platform/Windows-GNU.cmake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


Re: [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-25 Thread Petr Hosek via cfe-commits
Thanks for the heads up, this should be addressed
by c86f56e32e724c6018e579bb2bc11e667c96fc96, let me know if there are other
issues.

On Fri, Jul 24, 2020 at 12:33 PM Vedant Kumar via Phabricator <
revi...@reviews.llvm.org> wrote:

> vsk added a comment.
>
> @phosek I suspect this is causing a cmake error on the lldb standalone
> bot, would you mind taking a look?
>
> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake-standalone/1858/
>
>   CMake Error at
> /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/lib/cmake/llvm/AddLLVM.cmake:823
> (add_executable):
> Target "lit-cpuid" links to target "ZLIB::ZLIB" but the target was not
> found.  Perhaps a find_package() call is missing for an IMPORTED
> target, or
> an ALIAS target is missing?
>   Call Stack (most recent call first):
> cmake/modules/AddLLDB.cmake:165 (add_llvm_executable)
> utils/lit-cpuid/CMakeLists.txt:1 (add_lldb_executable)
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D79219/new/
>
> https://reviews.llvm.org/D79219
>
>
>
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-25 Thread Vedant Kumar via cfe-commits
Thanks, that addressed the issue.

> On Jul 24, 2020, at 1:41 PM, Petr Hosek  wrote:
> 
> Thanks for the heads up, this should be addressed by 
> c86f56e32e724c6018e579bb2bc11e667c96fc96, let me know if there are other 
> issues.
> 
> On Fri, Jul 24, 2020 at 12:33 PM Vedant Kumar via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> vsk added a comment.
> 
> @phosek I suspect this is causing a cmake error on the lldb standalone bot, 
> would you mind taking a look?
> 
> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake-standalone/1858/ 
> 
> 
>   CMake Error at 
> /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/lib/cmake/llvm/AddLLVM.cmake:823
>  (add_executable):
> Target "lit-cpuid" links to target "ZLIB::ZLIB" but the target was not
> found.  Perhaps a find_package() call is missing for an IMPORTED target, 
> or
> an ALIAS target is missing?
>   Call Stack (most recent call first):
> cmake/modules/AddLLDB.cmake:165 (add_llvm_executable)
> utils/lit-cpuid/CMakeLists.txt:1 (add_lldb_executable)
> 
> 
> Repository:
>   rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D79219/new/ 
> 
> https://reviews.llvm.org/D79219 
> 
> 
> 

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