From 003686fd6ae6c3ef026bff3e601010bdd3ad5f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Wed, 1 Jan 2025 16:25:26 +0100 Subject: [PATCH 1/2] [cmake] Extend zstd.dll finding logic from MSVC to WIN32 in general Extend the special logic for finding `zstd.dll` in `Findzstd` to apply to all Windows configurations rather than just MSVC. From what I understand, this naming scheme is specific to Windows in general, rather than "Windows with MSVC", and `.lib` files are always used to link to shared libraries. I could reproduce the original problem when using Clang with Conda-installed LLVM, and extending the logic to `WIN32` seems to fix it for me. That said, I'm not an expert on Windows and I have only done very limited testing, so I'd appreciate if someone could double check that I'm not breaking some workflow. Fixes #121345 --- llvm/cmake/modules/Findzstd.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/cmake/modules/Findzstd.cmake b/llvm/cmake/modules/Findzstd.cmake index 86b6d48b6ec6b..6726e9ea940b9 100644 --- a/llvm/cmake/modules/Findzstd.cmake +++ b/llvm/cmake/modules/Findzstd.cmake @@ -10,7 +10,7 @@ # zstd::libzstd_shared # zstd::libzstd_static -if(MSVC) +if(WIN32) set(zstd_STATIC_LIBRARY_SUFFIX "_static\\${CMAKE_STATIC_LIBRARY_SUFFIX}$") else() set(zstd_STATIC_LIBRARY_SUFFIX "\\${CMAKE_STATIC_LIBRARY_SUFFIX}$") @@ -33,7 +33,7 @@ if(zstd_FOUND) set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}") elseif (NOT TARGET zstd::libzstd_shared) add_library(zstd::libzstd_shared SHARED IMPORTED) - if(MSVC) + if(WIN32) include(GNUInstallDirs) # For CMAKE_INSTALL_LIBDIR and friends. # IMPORTED_LOCATION is the path to the DLL and IMPORTED_IMPLIB is the "library". get_filename_component(zstd_DIRNAME "${zstd_LIBRARY}" DIRECTORY) From 0e5ba65dad894d9ea0b3dfb8ded9d44551ded13a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Thu, 2 Jan 2025 11:25:58 +0100 Subject: [PATCH 2/2] [cmake] Use CMAKE_CXX_SIMULATE_ID to distinguish MSVC ABI in Findzstd Instead of unconditionally assuming MSVC-alike behavior for WIN32 that could negatively impact MSVC, use `CMAKE_CXX_SIMULATE_ID` to detect Clang using MSVC ABI. Note that this variable is unset for MSVC itself, so use it alternatively to MSVC. Thanks to @isuruf for the suggestion (https://github.com/conda-forge/llvmdev-feedstock/issues/306#issuecomment-2567306830). --- llvm/cmake/modules/Findzstd.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/cmake/modules/Findzstd.cmake b/llvm/cmake/modules/Findzstd.cmake index 6726e9ea940b9..f6ca5d1ebe546 100644 --- a/llvm/cmake/modules/Findzstd.cmake +++ b/llvm/cmake/modules/Findzstd.cmake @@ -10,7 +10,7 @@ # zstd::libzstd_shared # zstd::libzstd_static -if(WIN32) +if(MSVC OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC") set(zstd_STATIC_LIBRARY_SUFFIX "_static\\${CMAKE_STATIC_LIBRARY_SUFFIX}$") else() set(zstd_STATIC_LIBRARY_SUFFIX "\\${CMAKE_STATIC_LIBRARY_SUFFIX}$") @@ -33,7 +33,7 @@ if(zstd_FOUND) set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}") elseif (NOT TARGET zstd::libzstd_shared) add_library(zstd::libzstd_shared SHARED IMPORTED) - if(WIN32) + if(MSVC OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC") include(GNUInstallDirs) # For CMAKE_INSTALL_LIBDIR and friends. # IMPORTED_LOCATION is the path to the DLL and IMPORTED_IMPLIB is the "library". get_filename_component(zstd_DIRNAME "${zstd_LIBRARY}" DIRECTORY)