[MinGW][llvm-config] Use unversioned library name

[mati865]

Summary

When building LLVM for MinGW bare LLVM.dll or libLLVM.dll with https://reviews.llvm.org/D87517 is created

[mstorsjo]

llvm/tools/llvm-config/llvm-config.cpp

382–385

Shouldn't this be isWindowsGNUEnvironment()?

[mati865]

llvm/tools/llvm-config/llvm-config.cpp

382–385

Sorry, I did not get along with my editor when making last second changes.
I'll update diff later when builds finish.

[mstorsjo]

If the corresponding DLLs built in MSVC mode actually have a version number suffix, should we change the build for MinGW mode to include a number as well? Or is any of the relevant DLLs ever built in MSVC mode at all?

[mati865]

I don't use MSVC but apparently there is no libLLVM there: https://github.com/llvm/llvm-project/blob/9c651c231f3144f53e13cd0a1747589e1b2edccd/llvm/tools/llvm-shlib/CMakeLists.txt#L15

[mstorsjo]

In D87521#2269384, @mati865 wrote:

I don't use MSVC but apparently there is no libLLVM there: https://github.com/llvm/llvm-project/blob/9c651c231f3144f53e13cd0a1747589e1b2edccd/llvm/tools/llvm-shlib/CMakeLists.txt#L15

Right.. but, put another way - what code appends a version number in dynamic libs when built for other platforms, but decides not to, when targeting windows in general, or mingw specifically? (If there's a mingw-specific clause here, there should be a corresponding one in the cmakefiles somewhere - but I'm wondering if the right course of action would be to remove that mingw specific clause in cmakefiles, instead of adding a matching one here?)

[mati865]

what code appends a version number in dynamic libs when built for other platforms, but decides not to, when targeting windows in general, or mingw specifically?

I have very limited knowledge about what is going on but I think this version script controls it: https://github.com/llvm/llvm-project/blob/9c651c231f3144f53e13cd0a1747589e1b2edccd/llvm/tools/llvm-shlib/CMakeLists.txt#L51

[mstorsjo]

In D87521#2269446, @mati865 wrote:

what code appends a version number in dynamic libs when built for other platforms, but decides not to, when targeting windows in general, or mingw specifically?

I have very limited knowledge about what is going on but I think this version script controls it: https://github.com/llvm/llvm-project/blob/9c651c231f3144f53e13cd0a1747589e1b2edccd/llvm/tools/llvm-shlib/CMakeLists.txt#L51

The version script only affects how symbols are exported, but I can try to dig around to see where it comes from in the filename.

[mstorsjo]

Ok, so if I read code correctly, the suffix gets added here: https://github.com/llvm/llvm-project/blob/9c651c231f3144f53e13cd0a1747589e1b2edccd/llvm/cmake/modules/AddLLVM.cmake#L599-L606

So it gets added within an if (UNIX) condition. Hence one shouldn't expect version suffix on DLLs built for windows, regardless of MSVC vs mingw. The prebuilt installers of LLVM for windows (targeting MSVC) would confirm this; they contain LLVM-C.dll and LTO.dll.

However the code here in llvm-config seems to add LLVM_DYLIB_VERSION in the filenames, regardless of that logic. So the right path forward, if I understand things correctly, would be to stop adding LLVM_DYLIB_VERSION into the expected filenames for any windows target, not only mingw.

I had an old msvc based build lying around, and there I see the same, I have unsuffixed DLLs, and if trying to run e.g. llvm-config.exe --link-shared I'm getting llvm-config: error: LLVM-10svn.dll is missing. So this change, if applied to MSVC configurations, would be correct there as well.

@hans - Does that sound right to you?

[mati865]

Cygwin seems to be the only Windows target that adds version to the library names.

[mstorsjo]

In D87521#2270103, @mati865 wrote:

Cygwin seems to be the only Windows target that adds version to the library names.

Right - I guess cmake treats UNIX as true when targeting cygwin.

[hans]

@hans - Does that sound right to you?

Yes, that matches my experience. But I don't know the backstory here. Maybe it would make sense to have version suffixes also on Windows?

[mstorsjo]

In D87521#2270953, @hans wrote:

@hans - Does that sound right to you?

Yes, that matches my experience. But I don't know the backstory here. Maybe it would make sense to have version suffixes also on Windows?

I guess it would make sense - I don't see why it shouldn't be used there as well. There's probably some other unix-isms within the same if (UNIX block in cmake, but at least the version number aspect probably would make sense to use on windows on both mingw and msvc configurations.

llvm/tools/llvm-config/llvm-config.cpp

#endif
   /// in the first place. This can't be done at configure/build time.
 
   StringRef SharedExt, SharedVersionedExt, SharedDir, SharedPrefix, StaticExt,
       StaticPrefix, StaticDir = "lib";
   std::string DirSep = "/";
   const Triple HostTriple(Triple::normalize(LLVM_HOST_TRIPLE));
   if (HostTriple.isOSWindows()) {
     SharedExt = "dll";
+    if (HostTriple.isWindowsGNUEnvironment())
+      SharedVersionedExt = ".dll";
+    else
       SharedVersionedExt = LLVM_DYLIB_VERSION ".dll";
     if (HostTriple.isOSCygMing()) {
       StaticExt = "a";
       StaticPrefix = "lib";
     } else {
       StaticExt = "lib";
       DirSep = "\\";
       std::replace(ActiveObjRoot.begin(), ActiveObjRoot.end(), '/', '\\');
       std::replace(ActivePrefix.begin(), ActivePrefix.end(), '/', '\\');
#endif
   }
 
   const bool BuiltDyLib = !!LLVM_ENABLE_DYLIB;
 
   /// CMake style shared libs, ie each component is in a shared library.
   const bool BuiltSharedLibs = !!LLVM_ENABLE_SHARED;
 
   bool DyLibExists = false;
+  const std::string llvm_name =
+      HostTriple.isWindowsGNUEnvironment() ? "LLVM" : "LLVM-";
   const std::string DyLibName =
-      (SharedPrefix + "LLVM-" + SharedVersionedExt).str();
+      (SharedPrefix + llvm_name + SharedVersionedExt).str();
 
   // If LLVM_LINK_DYLIB is ON, the single shared library will be returned
   // for "--libs", etc, if they exist. This behaviour can be overridden with
   // --link-static or --link-shared.
   bool LinkDyLib = !!LLVM_LINK_DYLIB;
 
   if (BuiltDyLib) {
     std::string path((SharedDir + DirSep + DyLibName).str());