Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to build shared library on Windows (MinGW-w64) #2225

Closed
brechtsanders opened this issue Jun 1, 2024 · 9 comments · Fixed by #2229
Closed

Unable to build shared library on Windows (MinGW-w64) #2225

brechtsanders opened this issue Jun 1, 2024 · 9 comments · Fixed by #2229

Comments

@brechtsanders
Copy link

When building highway 1.2.0 on Windows using MinGW-w64 the shared build fails with the following error:

hwy/contrib/thread_pool/topology.cc:67:20: error: function 'bool hwy::HaveThreadingSupport()' definition is marked dllimport

When looking at this section in hwy/highway_export.h:

#if !defined(HWY_SHARED_DEFINE)
#define HWY_DLLEXPORT
#define HWY_CONTRIB_DLLEXPORT
#define HWY_TEST_DLLEXPORT
#else  // !HWY_SHARED_DEFINE

#ifndef HWY_DLLEXPORT
#if defined(hwy_EXPORTS)
/* We are building this library */
#ifdef _WIN32
#define HWY_DLLEXPORT __declspec(dllexport)
#else
#define HWY_DLLEXPORT __attribute__((visibility("default")))
#endif
...

and this section in CMakeLists.txt:

if("${HWY_LIBRARY_TYPE}" STREQUAL "SHARED")
  set(DLLEXPORT_TO_DEFINE "HWY_SHARED_DEFINE")
else()
  set(DLLEXPORT_TO_DEFINE "HWY_STATIC_DEFINE")
endif()

it's clear that set(DLLEXPORT_TO_DEFINE "HWY_SHARED_DEFINE") causes HWY_DLLEXPORT to be defined as nothing instead of __declspec(dllexport).

I was able to build successfully after changing CMakeLists.txt so HWY_SHARED_DEFINE is not defined.

@vtorri
Copy link

vtorri commented Jun 1, 2024

same here

@vtorri
Copy link

vtorri commented Jun 2, 2024

when I compile with verbose mode, what is passed to compiler is -Dhwy_contrib_EXPORTS not -Dhwy_EXPORTS. So, according to highway_export.h, HWY_DLLEXPORT is defined to __declspec(dllimport)

@brechtsanders
Copy link
Author

That is only true if HWY_SHARED_DEFINE is not defined, because if it is HWY_DLLEXPORT is empty, see the code from hwy/highway_export.h I mentioned above.

@kmilos
Copy link

kmilos commented Jun 2, 2024

#413 would still be nice to have 😉

@jan-wassenberg
Copy link
Member

Apologies, I used the wrong macro: it should have been HWY_CONTRIB_DLLEXPORT for anything in hwy/contrib. Fixing shortly.

That aside, I'm not sure it actually makes sense to use shared libraries for Highway. There's not a lot of code that can actually go into the .so/.dll, and maybe it's more trouble than it is worth?

@jan-wassenberg
Copy link
Member

@kmilos we can certainly add the LANGUAGES CXX to CMakelists, I'm not sure the other prefix change is still relevant because we now instead set RUNTIME_OUTPUT_DIRECTORY?

@kmilos
Copy link

kmilos commented Jun 3, 2024

I was suggesting at MSYS2 in the CI in general...

@jan-wassenberg
Copy link
Member

hm, when we last tried enabling it, there were numerous issues. Do you have any statistics on how many projects are using Highway with MSYS/MinGW? Would anyone be willing to help with these and future issues?

copybara-service bot pushed a commit that referenced this issue Jun 3, 2024
PiperOrigin-RevId: 639656599
@kmilos
Copy link

kmilos commented Jun 3, 2024

Do you have any statistics on how many projects are using Highway with MSYS/MinGW?

So far libjxl and libvips depend on it directly in the MSYS2 repo https://packages.msys2.org/package/mingw-w64-ucrt-x86_64-highway?repo=ucrt64 and then all of their clients. It's hard to tell who else uses it indirectly, outside the repo...

copybara-service bot pushed a commit that referenced this issue Jun 3, 2024
PiperOrigin-RevId: 639656599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants