src-d/wmd-relax

Compile errors with latest MS Build Tools in Windows

obbrc opened this issue · 7 comments

obbrc commented

The current pip installation gives several errors for windows 10 with latest MS Build Tools. This is because in VC 2015+, Universal CRT is updated, and looks like WMD code supports VC 2014. A quick glance at the files suggests the master also have the same issues. Below is how I made it work. Not sure if the solution is correct or not, but wanted to share.

  1. restrict references in *.h files need to be replaced by __restrict (see https://docs.microsoft.com/en-us/cpp/cpp/extension-restrict?view=vs-2015)
  2. reinsterpret_cast for PyLong_AsLong is causing problems. I used below code in python.cc line 152 and seems to be working. I'm not familiar with reinterpret_cast so this may cause memory isues.
if (cache_obj != Py_None) {
                 long  l=PyLong_AsLong(cache_obj);
                 cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>( &l ));
  1. setup.py windows compile flag for openmp should be updated with openmp:experimental for SIMD (see https://docs.microsoft.com/en-us/cpp/build/reference/openmp-enable-openmp-2-0-support?view=vs-2015)

With these changes WMD-Relax compiles, and gives the same results as Linux version.

Hi @obbrc

Thanks for the investigation.

Regarding restrict:

#ifdef _MSC_VER
#define restrict __restrict
#endif

Regarding reinterpret_cast, it casts a Python long to a pointer. I am not sure which problems you mention: it does not compile or it segfaults.

Let's have a PR with the Windows fixes?

obbrc commented

yeah, obviously your restrict solution is more elegant :)

reinterpret_cast gave below compile error.

python.cc(126): warning C4244: '=': conversion from 'npy_intp' to 'int', possible loss of data

python.cc(204): note: see reference to function template instantiation 'PyObject *emd_entry<`anonymous-namespace'::EMDRelaxedCache>(PyObject *,PyObject *,PyObject *,float (__cdecl *)(const float *,const float *,const float *,uint32_t,const C &))' being compiled

        with

        [

            C=`anonymous-namespace'::EMDRelaxedCache

       

python.cc(152): error C2440: 'reinterpret_cast': cannot convert from 'long' to 'intptr_t'

python.cc(152): note: Conversion is a valid standard conversion, which can be performed implicitly or by use of static_cast, C-style cast or function-style cas

 

The second error should be fixed by

#ifdef WIN32
#define PyLong_AsLong64(i) PyLong_AsLongLong(i)
#else
#define PyLong_AsLong64(i) PyLong_AsLong(i)
#endif

and replacing PyLong_AsLong on line 153 with PyLong_AsLong64.

I was wondering if this fix was checked in or was going to be checked in. I tried manually compiling on windows but get this after compiling:

image

I have it running on linux without issue (very awesome) but would like to get on windows as well. I have very little C experience so I imagine this is and issue on my side.

What I did to get to error above:

  1. git clone --recursive https://github.com/src-d/wmd-relax
  2. updated the .h , .cc and setup.py per this post
  3. cd wmd-relax
  4. python setup.py build
  5. python setup.py install

Build tools: VS 2019

The second error should be fixed by

#ifdef WIN32
#define PyLong_AsLong64(i) PyLong_AsLongLong(i)
#else
#define PyLong_AsLong64(i) PyLong_AsLong(i)
#endif

and replacing PyLong_AsLong on line 153 with PyLong_AsLong64.

Doing this did not work for me, but changing the code as in point 2 of the original post worked.

@arthuranderson14 for me, the issue was due to vcruntime140_1.dll missing. There's probably some Visual C++ Redistributable you're missing.

Please PR the fix somebody, I am not using Windows so cannot test myself.