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

now works on 32 bit arm platforms, such as RPi #18

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

Conversation

zertyz
Copy link

@zertyz zertyz commented Oct 1, 2018

No description provided.

Radfordhound added a commit to Radfordhound/HedgeLib that referenced this pull request Jan 3, 2020
flat_hash_map 32-bit fixes adapted from this PR: skarupke/flat_hash_map#18
@saierd
Copy link

saierd commented Feb 26, 2020

Thank you for your changes, I included them in my copy of the code.

I had a small problem, though. Your check for 32 or 64 bit systems does not handle AArch64 correctly, which leads to segmentation faults on that platform. Since quite some people seem to copy the changes from this pull request, you might want to add this fix.


// Check GCC
#if __GNUC__
#if __x86_64__ || __ppc64__
Copy link

Choose a reason for hiding this comment

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

Should be:

#if __x86_64__ || __ppc64__ || __aarch64__

william-silversmith added a commit to seung-lab/fastremap that referenced this pull request May 25, 2022
william-silversmith added a commit to seung-lab/fastremap that referenced this pull request May 25, 2022
* perf: switch to ska::flat_hash_map instead of std::unordered_map

Small ~10% improvement to connectomics.npy renumber, but ~45%
improvement to a random array.

* fix: support 32-bit

skarupke/flat_hash_map#18

* fix: allocate default table

Avoids problems with dynamic library situations:
skarupke/flat_hash_map#26

* fix: use more compatible definition of void_t

* docs: update comment date
@@ -4,6 +4,24 @@

#pragma once

// Check windows
#if _WIN32 || _WIN64
Copy link

Choose a reason for hiding this comment

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

Suggested change
#if _WIN32 || _WIN64
#if SIZE_MAX == UINT64_MAX

same goes for every other place

Copy link

@ya-solnyshko ya-solnyshko left a comment

Choose a reason for hiding this comment

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

This code:

    int8_t next_size_over(size_t & size) const
    {
        size = std::max(size_t(2), detailv3::next_power_of_two(size));
        return 64 - detailv3::log2(size);
    }
    void commit(int8_t shift)
    {
        this->shift = shift;
    }
    void reset()
    {
        shift = 63;
    }
private:
    int8_t shift = 63;

also shall be adapted for 32 bits, because shift by more than 32 bits is UB

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.

4 participants