mstorsjo added a comment.
Super-nitpick: If you want to capitalize mingw, it's MinGW (minimalist gnu for
windows) 😛 But as that's rather annoying to type, the all-lowercase version is
quite fine as well.
================
Comment at: clang/tools/libclang/CMakeLists.txt:71
+ list(APPEND LIBS ${CMAKE_DL_LIBS})
endif()
----------------
If you say this is the same way it's done elsewhere, then sure - although I
have no idea about what the issue is, why I haven't run into it, etc. Normally
you wouldn't have a `libdl` on mingw right? What's the concrete issue you're
running into, and in which conditions would one run into it?
================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1872
+ if(CMAKE_HOST_UNIX AND NOT MINGW)
set(dest_binary "$<TARGET_FILE_NAME:${target}>")
endif()
----------------
Not entirely sure what this one does - is it just a condition that needs to
match the one for `create_symlink` below?
================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1898
+ if(CMAKE_HOST_UNIX AND NOT MINGW)
set(LLVM_LINK_OR_COPY create_symlink)
else()
----------------
What's the practical issue you're trying to fix with this one here? If
`CMAKE_HOST_UNIX`, i.e. when cross compiling, it does work fine to create
symlinks (saving a bit of disk space and bandwidth). Then when transferring the
built products to an actual windows system, they're converted into copies at
some point (e.g. when zipping up the results).
================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:957
+ #
+ # FIXME; mingw lld driver doesn't support any of the options below
if(APPLE)
----------------
We could certainly add support for it in the mingw driver - ideally with the
same name as for ELF, which then would be remapped to the corresponding
lld-link option. This takes just a couple lines in lld/MinGW/Options.td,
lld/MinGW/Driver.cpp and lld/test/MinGW/driver.test.
================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:967
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
- elseif(LINKER_IS_LLD_LINK)
+ elseif(LINKER_IS_LLD_LINK AND NOT MINGW)
append("/lldltocache:${PROJECT_BINARY_DIR}/lto.cache"
----------------
Do you happen to know why `LINKER_IS_LLD_LINK` gets set in this case? `ld.lld`
(the ELF linker interface, which then the MinGW driver remaps onto the COFF
backend with the `lld-link` interface) certainly doesn't take `lld-link` style
options. I believe (without diving further into it) that we shouldn't be
setting this flag in this combination, but with the option implemented, we
should fit it into the case further above, `elseif((UNIX OR MINGW) AND
LLVM_USE_LINKER STREQUAL "lld")`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80425/new/
https://reviews.llvm.org/D80425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
