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.

Sign up for GitHub

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

Fix Windows compile issues #67

Closed
wants to merge 3 commits into from
Closed

Fix Windows compile issues #67

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bd1bf3
Fix Windows compile issues
scott-8 Apr 2, 2020
81132cc
Update emd.h
scott-8 Apr 2, 2020
1e65779
Update emd_relaxed.h
scott-8 Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions emd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// fix windows compile
#ifdef _MSC_VER
scott-8 marked this conversation as resolved.
Show resolved Hide resolved
#define __restrict__ __restrict
#endif
// fix inttypes for GCC
#ifndef __STDC_FORMAT_MACROS
#define __STDC_FORMAT_MACROS
Expand Down
5 changes: 5 additions & 0 deletions emd_relaxed.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// fix windows compile
#ifdef _MSC_VER
scott-8 marked this conversation as resolved.
Show resolved Hide resolved
#define __restrict__ __restrict
#endif

#pragma once

#include <cstdint>
Expand Down
4 changes: 2 additions & 2 deletions python.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ PyObject* emd_entry(
C *cache = nullptr;
std::unique_ptr<C> cache_ptr;
if (cache_obj != Py_None) {
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>(
PyLong_AsLong(cache_obj)));
long l=PyLong_AsLong(cache_obj);
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>( &l ));
Comment on lines +152 to +153
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no good: instead of converting an integer to a cache pointer you pretend that a local variable's address is a cache pointer. The size of long on Windows equals to 4 bytes while the pointer size is 8 bytes. Can you please replace PyLong_AsLong with PyLong_AsLongLong here?

Suggested change
long l=PyLong_AsLong(cache_obj);
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>( &l ));
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>(
PyLong_AsLongLong(cache_obj)));

Copy link
Author

@scott-8 scott-8 Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to get this to work. I continue to get python.cc(152): error C2440: 'reinterpret_cast': cannot convert from '__int64' to 'intptr_t'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed reinterpret_cast<intptr_t>(PyLong_AsLongLong(cache_obj)) to reinterpret_cast<intptr_t *>(PyLong_AsLongLong(cache_obj)) and it compiled successfully, I'm not exactly sure about the implications of that, though.

if (PyErr_Occurred()) {
return NULL;
}
Expand Down
6 changes: 5 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"-Wno-sign-compare", "-fPIC", "-flto"],
"Linux": ["-fopenmp", "-std=c++11", "-march=native", "-ftree-vectorize",
"-DNDEBUG", "-Wno-sign-compare", "-fPIC", "-flto"],
"Windows": ["/openmp", "/std:c++latest", "/arch:AVX2", "/DNDEBUG", "/LTCG",
"Windows": ["/openmp:experimental", "/std:c++latest", "/arch:AVX2", "/DNDEBUG", "/LTCG",
"/GL"]
}

Expand All @@ -29,6 +29,10 @@
"Windows": ["/LTCG", "/GL"]
}

if platform.system() == "Windows" and platform.python_version_tuple() >= ("3", "5", "0"):
CXX_FLAGS["Windows"].append("/d2FH4-")
LD_FLAGS["Windows"].append("/d2:-FH4-")

setup(
name=PACKAGE,
description="Accelerated functions to calculate Word Mover's Distance",
Expand Down