mreineck / pocketfft

Fork of https://gitlab.mpcdf.mpg.de/mtr/pocketfft to simplify external contributions
BSD 3-Clause "New" or "Revised" License
75 stars 34 forks source link

Compilation fails when cross compiling for windows #5

Closed MCredstoner2004 closed 2 years ago

MCredstoner2004 commented 2 years ago

I'm working on a project that uses C++20 features and uses this library, I primarily develop under linux but like testing windows support from time to time. When trying to crosscompile for windows using x86_64-w64-mingw32, I get the following error (even when compiling the example):

error: ‘::aligned_alloc’ has not been declared; did you mean ‘pocketfft::detail::aligned_alloc’?
  156 |   void *ptr = ::aligned_alloc(align,(size+align-1)&(~(align-1)));

After looking into it, this seems to be due to the fact that windows doesn't support said function, despite it being part of the C++17 standard (as detailed here). So simply checking for __cplusplus >= 201703L isn't enough for guaranteeing the existence of said function. I also tested manually forcing the use of the portable version of the function and both the example and my project worked flawlessly under windows. I think the solution should be as simple as checking for either the mingw specific macro __MINGW32__ or for windows altogether with _WIN32.

mreineck commented 2 years ago

Thanks for the report!

I think the solution should be as simple as checking for either the mingw specific macro MINGW32 or for windows altogether with _WIN32.

Hmm, checking for __MINGW32__ is probably too weak, since the problem will probably also occur when compiling with MSVC, correct?

On the other hand, I know that https://gitlab.mpcdf.mpg.de/mtr/ducc compiles fine with MSVC, even though I have the same macros in src/ducc0/infra/aligned_array.h. Strange...

Rather than adding more guesswork to the code, would it be an acceptable solution for you if I introduced a macro called POCKETFFT_USE_PORTABLE_ALLOC that switches to the portable version unconditionally?

MCredstoner2004 commented 2 years ago

Hmm, checking for __MINGW32__ is probably too weak, since the problem will probably also occur when compiling with MSVC, correct?

On the other hand, I know that https://gitlab.mpcdf.mpg.de/mtr/ducc compiles fine with MSVC, even though I have the same macros in src/ducc0/infra/aligned_array.h. Strange...

Ah, that's because MSVC doesn't define the correct macros, at least according to this so checking for MinGW should be enough

Rather than adding more guesswork to the code, would it be an acceptable solution for you if I introduced a macro called POCKETFFT_USE_PORTABLE_ALLOC that switches to the portable version unconditionally?

However, this is also acceptable and I could add the neccessary logic during the configure step

mreineck commented 2 years ago

Ah, that's because MSVC doesn't define the correct macros, at least according to this so checking for MinGW should be enough

Oh wow, thanks for pointing that out! (I never have direct contact with the compiler, I just see whether CI succeeds or fails, so I never made detailed experiments.)

In this case I think it's fine to try the MINGW version.

So we should replace

#if __cplusplus >= 201703L

with

#if (__cplusplus >= 201703L) && (!defined(__MINGW32__))

correct?