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

Compile errors with latest MS Build Tools in Windows #61

Open
obbrc opened this issue Sep 12, 2019 · 7 comments
Open

Compile errors with latest MS Build Tools in Windows #61

obbrc opened this issue Sep 12, 2019 · 7 comments

Comments

@obbrc
Copy link

obbrc commented Sep 12, 2019

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.

@vmarkovtsev
Copy link
Collaborator

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
Copy link
Author

obbrc commented Sep 15, 2019

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

 

@vmarkovtsev
Copy link
Collaborator

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.

@arthuranderson14
Copy link

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

@scott-8
Copy link

scott-8 commented Apr 1, 2020

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.

@scott-8
Copy link

scott-8 commented Apr 1, 2020

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

@vmarkovtsev
Copy link
Collaborator

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

scott-8 added a commit to scott-8/wmd-relax that referenced this issue Apr 2, 2020
Fixes applied based on discussions here:
src-d#61
mkleehammer/pyodbc#663
scott-8 added a commit to scott-8/wmd-relax that referenced this issue Apr 2, 2020
Fixes applied based on discussions here:
src-d#61
mkleehammer/pyodbc#663

Signed-off-by: Scott <13770321+scott-8@users.noreply.github.com>
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

No branches or pull requests

4 participants