mati865 added inline comments.
================ 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" ---------------- mstorsjo wrote: > mati865 wrote: > > thieta wrote: > > > mstorsjo wrote: > > > > 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")` > > > Yeah I bet that variable is set because I pass `LLVM_USE_LINKER=lld` but > > > I haven't digged to deeply. I can rework the if statement here when we > > > have the lld option in there. > > > Yeah I bet that variable is set because I pass LLVM_USE_LINKER=lld but I > > > haven't digged to deeply. > > > > It does use `lld-link` when you use `LLVM_USE_LINKER=lld`. > > > > The problem lies in this line: > > ``` > > append("/lldltocache:${PROJECT_BINARY_DIR}/lto.cache" > > ``` > > For MinGW that should read: > > ``` > > append("-Wl,/lldltocache:${PROJECT_BINARY_DIR}/lto.cache" > > ``` > > That's because you are passing this flag to GCC/Clang. > > For MinGW that should read: > > > > append("-Wl,/lldltocache:${PROJECT_BINARY_DIR}/lto.cache" > > We're adding this option properly in the mingw frontend, so we shouldn't do > the hacky way of passing lld-link options via the mingw frontend by passing > them as `/option`. > > And the reason `LINKER_IS_LLD_LINK` is set seems to be this: > > ``` > if(CMAKE_LINKER MATCHES "lld-link" OR (WIN32 AND LLVM_USE_LINKER STREQUAL > "lld") OR LLVM_ENABLE_LLD) > ``` > > Perhaps that should be changed into > > ``` > if(CMAKE_LINKER MATCHES "lld-link" OR (WIN32 AND LLVM_USE_LINKER STREQUAL > "lld" AND NOT MINGW) OR LLVM_ENABLE_LLD) > ``` > > We're adding this option properly in the mingw frontend, so we shouldn't do > the hacky way of passing lld-link options via the mingw frontend by passing > them as /option. I was about to add that and one more LTO related option but thought "what if LLVM should link with lld-link on MinGW?" and eventually forgot about it. > And the reason LINKER_IS_LLD_LINK is set seems to be this: > ``` > if(CMAKE_LINKER MATCHES "lld-link" OR (WIN32 AND LLVM_USE_LINKER STREQUAL > "lld") OR LLVM_ENABLE_LLD) > ``` > Perhaps that should be changed into > ``` > if(CMAKE_LINKER MATCHES "lld-link" OR (WIN32 AND LLVM_USE_LINKER STREQUAL > "lld" AND NOT MINGW) OR LLVM_ENABLE_LLD) > ``` It won't work if somebody uses `LLVM_ENABLE_LLD=ON` instead of `LLVM_USE_LINKER=lld` which is supposed to be equivalent. That raises question how `LLVM_ENABLE_LLD=ON` does even work on UNIX platforms. IMO this would be better: ``` if(CMAKE_LINKER MATCHES "lld-link" OR MSVC AND (LLVM_USE_LINKER STREQUAL "lld" OR LLVM_ENABLE_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