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

cpython to pyd binary - "long" converted to "int" badly before shift / division operator #2670

Closed
gui36 opened this issue Oct 19, 2018 · 13 comments
Milestone

Comments

@gui36
Copy link

gui36 commented Oct 19, 2018

cython 0.29
python 3.6.6
compiler mingw-w64
os windows 7

command used:
python setup.py build_ext --inplace

setup.py:

from distutils.core import setup
from distutils.extension import Extension
from Cython.Build import cythonize
extensions = [Extension("shiftoperator", ['shiftoperator.py']]
setup(
    ext_modules=cythonize(extensions)
)

shiftoperator.py:

#!python
#cython: language_level=3
def longtoint():
    value = 80082 # int using more than 2 bytes == long
    print(value)
    shiftedby3 = value>>3
    dividedby8 = value//8
    print(shiftedby3)
    print(dividedby8)
    shiftedby3 = 80082>>3
    dividedby8 = 80082//8
    print(shiftedby3)
    print(dividedby8)

test_shift.py

import shiftoperator
shiftoperator.longtoint()

with cpython shiftoperator.py library used:

C:\scripts\tests>python test_shift.py
80082
10010
10010
10010
10010

with binary shiftoperator.cp36-win_amd64.pyd used:

C:\scripts\tests>python test_shift.py
80082
1818
1818
10010
10010

it seems the higher bit (65536 before shifting / 8192 after shifting) is dropped before the shifting process is done in case original long is saved on a variable

Can you check?
Thks

@gui36 gui36 changed the title cpython to pyd binary - "long" converted to "int" badly after shift / division operator cpython to pyd binary - "long" converted to "int" badly before shift / division operator Oct 19, 2018
scoder added a commit that referenced this issue Oct 20, 2018
scoder added a commit that referenced this issue Oct 20, 2018
@scoder
Copy link
Contributor

scoder commented Oct 20, 2018

Thanks for the report. However, I cannot reproduce it, neither locally nor on Windows. Might be related to mingw specifically. The 15 bit boundary is indeed special for PyLong integers, which can use 15 or 30 bit digits internally. I've seen mismatches between the CPython build and extension builds before, where CPython was using a different PyLong digit size than what the extension module build assumed. If that happens, there isn't really something Cython can do about it. It's basically a broken build setup. Like trying to compose an application of C code that assumes a 32bit int with code based on 16bit ints.

Could you verify that the test I wrote in #2672 fails for you?
Branch: https://github.com/cython/cython/tree/gh2670_win_intop
python3 runtests.py -vv cintop buildenv

Please also report the output of the buildenv test on the system where your example fails.

As a work-around, you can disable Cython's usage of the PyLong type internals by setting the C macro CYTHON_USE_PYLONG_INTERNALS=0. That slows down integer operations somewhat, but should make things working again.

@gui36
Copy link
Author

gui36 commented Oct 22, 2018

Hi

Thks for the support and sorry for the late reply. I will try to reply faster next time :)

Could you give example with the command?
What am I suppose to put as "buildenv"? mingw32?

Anyway, the command fails whatever the argument passed:

C:\scripts2\cython-gh2670_win_intop>python runtests.py --v cintop mingw32
runtests.py:348: UserWarning: Unable to find the c compiler: No such file or dir
ectory: msvc
  (language, os.strerror(sys.exc_info()[1].errno), cc))
runtests.py:348: UserWarning: Unable to find the cpp compiler: No such file or d
irectory: msvc
  (language, os.strerror(sys.exc_info()[1].errno), cc))
Python 3.6.6 (v3.6.6:4cf1f54eb7, Jun 27 2018, 03:37:03) [MSC v.1900 64 bit (AMD6
4)]

Running tests against Cython 0.29
Traceback (most recent call last):
  File "runtests.py", line 2509, in <module>
    main()
  File "runtests.py", line 2157, in main
    _, stats, return_code = runtests(options, cmd_args, coverage)
  File "runtests.py", line 2305, in runtests
    pyxbuild_dir=os.path.join(WORKDIR, "support"))
  File "C:\scripts2\cython-gh2670_win_intop\pyximport\pyxbuild.py", line 102, in
 pyx_to_dll
    dist.run_commands()
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\d
ist.py", line 955, in run_commands
    self.run_command(cmd)
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\d
ist.py", line 974, in run_command
    cmd_obj.run()
  File "C:\scripts2\cython-gh2670_win_intop\Cython\Distutils\old_build_ext.py",
line 186, in run
    _build_ext.build_ext.run(self)
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\c
ommand\build_ext.py", line 339, in run
    self.build_extensions()
  File "C:\scripts2\cython-gh2670_win_intop\Cython\Distutils\old_build_ext.py",
line 194, in build_extensions
    self.build_extension(ext)
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\c
ommand\build_ext.py", line 533, in build_extension
    depends=ext.depends)
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\_
msvccompiler.py", line 345, in compile
    self.initialize()
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\_
msvccompiler.py", line 238, in initialize
    vc_env = _get_vc_env(plat_spec)
  File "C:\Users\aaa\AppData\Local\Programs\Python\Python36\lib\distutils\_
msvccompiler.py", line 134, in _get_vc_env
    raise DistutilsPlatformError("Unable to find vcvarsall.bat")
distutils.errors.DistutilsPlatformError: Unable to find vcvarsall.bat

I can see using print that I get the default_compiler() = msvc using your script, but it is not installed on my machine.

    if language == 'cpp':
        cc = sysconfig.get_config_var('CXX')
    else:
        cc = sysconfig.get_config_var('CC')
    if not cc:
       cc = ccompiler.get_default_compiler()
    if not cc:
        return ''

MinGW-W64 can be found here:
https://sourceforge.net/projects/mingw-w64/

output that might be useful to you:

C:\scripts>gcc --version
gcc (x86_64-win32-seh-rev0, Built by MinGW-W64 project) 8.1.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
C:\scripts>gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=C:/Program\ Files/mingw-w64/x86_64-8.1.0-win32-seh-rt_v6-rev
0/mingw64/bin/../libexec/gcc/x86_64-w64-mingw32/8.1.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../../../src/gcc-8.1.0/configure --host=x86_64-w64-mingw32 --bu
ild=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --prefix=/mingw64 --with-sysr
oot=/c/mingw810/x86_64-810-win32-seh-rt_v6-rev0/mingw64 --enable-shared --enable
-static --disable-multilib --enable-languages=c,c++,fortran,lto --enable-libstdc
xx-time=yes --enable-threads=win32 --enable-libgomp --enable-libatomic --enable-
lto --enable-graphite --enable-checking=release --enable-fully-dynamic-string --
enable-version-specific-runtime-libs --disable-libstdcxx-pch --disable-libstdcxx
-debug --enable-bootstrap --disable-rpath --disable-win32-registry --disable-nls
--disable-werror --disable-symvers --with-gnu-as --with-gnu-ld --with-arch=noco
na --with-tune=core2 --with-libiconv --with-system-zlib --with-gmp=/c/mingw810/p
rerequisites/x86_64-w64-mingw32-static --with-mpfr=/c/mingw810/prerequisites/x86
_64-w64-mingw32-static --with-mpc=/c/mingw810/prerequisites/x86_64-w64-mingw32-s
tatic --with-isl=/c/mingw810/prerequisites/x86_64-w64-mingw32-static --with-pkgv
ersion='x86_64-win32-seh-rev0, Built by MinGW-W64 project' --with-bugurl=https:/
/sourceforge.net/projects/mingw-w64 CFLAGS='-O2 -pipe -fno-ident -I/c/mingw810/x
86_64-810-win32-seh-rt_v6-rev0/mingw64/opt/include -I/c/mingw810/prerequisites/x
86_64-zlib-static/include -I/c/mingw810/prerequisites/x86_64-w64-mingw32-static/
include' CXXFLAGS='-O2 -pipe -fno-ident -I/c/mingw810/x86_64-810-win32-seh-rt_v6
-rev0/mingw64/opt/include -I/c/mingw810/prerequisites/x86_64-zlib-static/include
-I/c/mingw810/prerequisites/x86_64-w64-mingw32-static/include' CPPFLAGS=' -I/c/
mingw810/x86_64-810-win32-seh-rt_v6-rev0/mingw64/opt/include -I/c/mingw810/prere
quisites/x86_64-zlib-static/include -I/c/mingw810/prerequisites/x86_64-w64-mingw
32-static/include' LDFLAGS='-pipe -fno-ident -L/c/mingw810/x86_64-810-win32-seh-
rt_v6-rev0/mingw64/opt/lib -L/c/mingw810/prerequisites/x86_64-zlib-static/lib -L
/c/mingw810/prerequisites/x86_64-w64-mingw32-static/lib '
Thread model: win32
gcc version 8.1.0 (x86_64-win32-seh-rev0, Built by MinGW-W64 project)

output of my command being build:

C:\scripts2\tests>python setup.py build_ext --force --inplace
running build_ext
building 'shiftoperator' extension
C:\Program Files\mingw-w64\x86_64-8.1.0-win32-seh-rt_v6-rev0\mingw64\bin\gcc.exe
 -mdll -O -Wall -IC:\Users\aaa\AppData\Local\Programs\Python\Python36\inclu
de -IC:\Users\aaa\AppData\Local\Programs\Python\Python36\include -c shiftop
erator.c -o build\temp.win-amd64-3.6\Release\shiftoperator.o
writing build\temp.win-amd64-3.6\Release\shiftoperator.cp36-win_amd64.def
C:\Program Files\mingw-w64\x86_64-8.1.0-win32-seh-rt_v6-rev0\mingw64\bin\gcc.exe
 -shared -s build\temp.win-amd64-3.6\Release\shiftoperator.o build\temp.win-amd6
4-3.6\Release\shiftoperator.cp36-win_amd64.def -LC:\Users\aaa\AppData\Local
\Programs\Python\Python36\libs -LC:\Users\aaa\AppData\Local\Programs\Python
\Python36\PCbuild\amd64 -lpython36 -lvcruntime140 -o C:\scripts2\tests\shiftoper
ator.cp36-win_amd64.pyd

In order to make it working, i do the following steps:
1/ generate libvcruntime140.a

 cd C:\Users\xxx\AppData\Local\Programs\Python\Python36\libs
gendef ../vcruntime140.def
dlltool --dllname ../vcruntime140.dll --def vcruntime140.def --output-lib libvcruntime140.a

2/ put the default compiler as mingw32 adding in distutils folder distutils.cfg as followed:

[build]
compiler = mingw32

3/ Editing cygwinccompiler.py on line 85 adding the following line:

        elif msc_ver == '1900':
            # Visual Studio 2015 / Visual C++ 14.0
            # "msvcr140.dll no longer exists"
            return ['vcruntime140']

@gui36
Copy link
Author

gui36 commented Oct 22, 2018

here the buildenv.c on my computer, result of
python -m cython -3 buildenv.pyx
buildenv.zip

@ducminh-phan
Copy link

@gui36 I have a similar issue with my code and had a discussion in the cython-users Google group. The problem is in the header pyport.h, in which SIZEOF_VOID_P is defined based on whether MS_WIN64 is defined or not. The solution is to pass -DMS_WIN64 when compiling with MinGW.

@gui36
Copy link
Author

gui36 commented Oct 23, 2018

Hi Ducminh-phan,

Indeed, it worked adding -DMS_WIN64 to the gcc command
Thks a lot! :)

Do you know how to add this argument to cythonizer?
Is there any file to modify with this argument?

@ducminh-phan
Copy link

@gui36 You just need to add -DMS_WIN64 to the build command: python setup.py build_ext --inplace - DMS_WIN64

@gui36
Copy link
Author

gui36 commented Oct 24, 2018

perfect thks!

@scoder scoder closed this as completed in 55861ed Oct 27, 2018
scoder added a commit that referenced this issue Oct 30, 2018
@scoder scoder added this to the 0.28.6 milestone Nov 1, 2018
scoder pushed a commit that referenced this issue Nov 1, 2018
@filips123
Copy link

@ducminh-phan Although this is quite old issue, I still had same problems. Adding -DMS_WIN64 fixed the issue for me. So, would it be possible to automatically detect usage of mingw32 and automatically pass -DMS_WIN64?

@filips123
Copy link

Also, I have another question. How to pass -DMS_WIN64 when using cythonize command?

@scoder
Copy link
Contributor

scoder commented Dec 21, 2019

How to pass -DMS_WIN64 when using cythonize command?

It should still work to add it to the CFLAGS.

@filips123
Copy link

Could Cython/compiler be set up to automatically include -DMS_WIN64 if needed?

@scoder
Copy link
Contributor

scoder commented Dec 23, 2019

Two problems here. a) Cython does not need to be involved in the C compilation at all, and thus may not be in a position to add the compile flag "automatically", and b) the user might have a different idea about what "is needed" than Cython, and Cython shouldn't get in the way in that case.

I don't think that Cython should do this.

@filips123
Copy link

Is there any other possibility for building Cython packages with MinGW? Or maybe official Python team needs to support something?

As MinGW is the only lightweight compiler, in comparison to VS Build Tools, it would be very useful to support it.

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