[PATCH] D134165: [llvm] prefix linker flag on non-MSVC compilers with `-Wl,`

[Ashay Rane via Phabricator via llvm-commits]

ashay-github added inline comments.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:133
       set(export_file_linker_flag "/DEF:\"${export_file_linker_flag}\"")
+    elseif(WIN32)
+      # If we're compiling on Windows and the compiler is not Visual Studio,
----------------
mstorsjo wrote:
> rnk wrote:
> > ashay-github wrote:
> > > beanz wrote:
> > > > Does the same issue occur if you use compilers other than clang? I'm wondering if this change should be clang-specific.
> > > Since both gcc and clang use `-Wl` to pass flags to the linker, I imagine that this problem is not clang specific. Unfortunately, I don't have gcc/MinGW setup to try this out.
> > I think if one configured a cmake project with clang-cl, CMake would invoke the linker directly, and it would go down the `if (MSVC)` codepath above. The linker would directly understand the /def flag.
> I’m not so sure about this change. If building in mingw mode, the linker doesn’t support any ‘/DEF:` flag at all, in pretty sure that this will break such builds.
> 
> So can you clarify, in exactly what setup does `if(MSVC)` not trigger, but you still have got an msvc style linker that supports the `/DEF:` option?
> So can you clarify, in exactly what setup does if(MSVC) not trigger, but you still have got an msvc style linker that supports the /DEF: option?

I see the error when I compile LLVM using clang.exe that comes bundled with Microsoft Visual Studio 2022. If I were to invoke `cl.exe`, then `.def` files are sent to the linker instead of being interpreted by the compiler.

It looks like `ld` can handle `.def` files without any special flags [https://sourceware.org/binutils/docs/ld/WIN32.html#index-using-a-DEF-file], so I think I'd need to add an exception for gcc so as to not break MinGW builds.  Does that seem right to you?  Oh, and thanks for catching this, @mstorsjo!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134165