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

fix 32-bit usage of size_t and support non-intel arches #207

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drew-parsons
Copy link

On 32-bit systems size_t is identical unsigned in, causing redefinition errors in tables (Array_I_S).

This patch guards against the 32-bit redefinition and defines
Array_I_S = Array_I_U for the python interface if not already defined.

Applies debian patch size_t_int32.patch
https://salsa.debian.org/science-team/netgen/-/blob/d7ca1c564d90d00ce3d83e0b63c36fbec11cf1ce/debian/patches/size_t_int32.patch

Also applies debian patch support-non-x86.patch
https://salsa.debian.org/science-team/netgen/-/blob/d7ca1c564d90d00ce3d83e0b63c36fbec11cf1ce/debian/patches/support-non-x86.patch
to support non-intel archictectures missing RDTSC.

Fixes #168

On 32-bit systems size_t is identical unsigned in, causing
redefinition errors in tables (Array_I_S).

This patch guards against the 32-bit redefinition and defines
Array_I_S = Array_I_U for the python interface if not already defined.

Applies debian patch size_t_int32.patch
https://salsa.debian.org/science-team/netgen/-/blob/d7ca1c564d90d00ce3d83e0b63c36fbec11cf1ce/debian/patches/size_t_int32.patch

Fixes NGSolve#168
Extends support of non-intel architectures which do not have an RDTSC
timer.

Applies debian patch support-non-x86.patch
https://salsa.debian.org/science-team/netgen/-/blob/d7ca1c564d90d00ce3d83e0b63c36fbec11cf1ce/debian/patches/support-non-x86.patch
Copy link
Contributor

@StefanBruens StefanBruens left a comment

Choose a reason for hiding this comment

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

Effectively, this just replaces the #else branch with using std::chrono

Comment on lines +22 to 26
#define NGCORE_HAVE_RDTSC
#elif defined __SSE__
#define NGCORE_HAVE_RDTSC
#include <x86intrin.h> // for __rdtsc() CPU time step counter
#endif // WIN32
Copy link
Contributor

Choose a reason for hiding this comment

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

same define in both branches ...

@@ -65,7 +67,7 @@ namespace ngcore
{
#if defined(__APPLE__) && defined(NETGEN_ARCH_ARM64)
return mach_absolute_time();
#elif defined(NETGEN_ARCH_AMD64)
#elif defined(NETGEN_ARCH_AMD64) || defined(NGCORE_HAVE_RDTSC)
Copy link
Contributor

Choose a reason for hiding this comment

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

change has no effect, NGCORE_HAVE_RDTSC is only ever defined if NETGEN_ARCH_AMD64 is defined.

@@ -74,6 +76,8 @@ namespace ngcore
return tics;
#elif defined(__EMSCRIPTEN__)
return std::chrono::high_resolution_clock::now().time_since_epoch().count();
#elifndef NGCORE_HAVE_RDTSC
Copy link
Contributor

Choose a reason for hiding this comment

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

Always true ...

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

Successfully merging this pull request may close these issues.

table.hpp:106 error: redefinition of ‘size_t* TablePrefixSum on 32-bit architectures
2 participants