efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.61k stars 275 forks source link

32-bit unit_tests errors and compiler warnings #143

Closed andreasbaumann closed 2 years ago

andreasbaumann commented 2 years ago
  REQUIRE(UnitTestInternalAccess::reserve_calc<IntIntTable>(
              (1ULL << 31) * slot_per_bucket) == 31);
  REQUIRE(UnitTestInternalAccess::reserve_calc<IntIntTable>(
              ((1ULL << 31) + 1) * slot_per_bucket) == 32);

  REQUIRE(UnitTestInternalAccess::reserve_calc<IntIntTable>(
              (1ULL << 61) * slot_per_bucket) == 61);
  REQUIRE(UnitTestInternalAccess::reserve_calc<IntIntTable>(
              ((1ULL << 61) + 1) * slot_per_bucket) == 62);

results (when compiling) in:

/build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:49:28: warning: conversion from ‘long long unsigned int’ to ‘size_t’ {aka ‘unsigned int’} changes value from ‘8589934592’ to ‘0’ [-Woverflow]
   49 |               (1ULL << 31) * slot_per_bucket) == 31);
      |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:51:34: warning: conversion from ‘long long unsigned int’ to ‘size_t’ {aka ‘unsigned int’} changes value from ‘8589934596’ to ‘4’ [-Woverflow]
   51 |               ((1ULL << 31) + 1) * slot_per_bucket) == 32);
      |               ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:54:28: warning: conversion from ‘long long unsigned int’ to ‘size_t’ {aka ‘unsigned int’} changes value from ‘9223372036854775808’ to ‘0’ [-Woverflow]
   54 |               (1ULL << 61) * slot_per_bucket) == 61);
      |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:56:34: warning: conversion from ‘long long unsigned int’ to ‘size_t’ {aka ‘unsigned int’} changes value from ‘9223372036854775812’ to ‘4’ [-Woverflow]
   56 |               ((1ULL << 61) + 1) * slot_per_bucket) == 62);
      |               ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

and when running:

1: /build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:48: FAILED:
1:   REQUIRE( UnitTestInternalAccess::reserve_calc<IntIntTable>( (1ULL << 31) * slot_per_bucket) == 31 )
1: with expansion:
1:   0 == 31
1: /build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:50: FAILED:
1:   REQUIRE( UnitTestInternalAccess::reserve_calc<IntIntTable>( ((1ULL << 31) + 1) * slot_per_bucket) == 32 )
1: with expansion:
1:   0 == 32
1: /build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:53: FAILED:
1:   REQUIRE( UnitTestInternalAccess::reserve_calc<IntIntTable>( (1ULL << 61) * slot_per_bucket) == 61 )
1: with expansion:
1:   0 == 61
1: /build/libcuckoo/src/libcuckoo-0.3/tests/unit-tests/test_resize.cc:55: FAILED:
1:   REQUIRE( UnitTestInternalAccess::reserve_calc<IntIntTable>( ((1ULL << 61) + 1) * slot_per_bucket) == 62 )
1: with expansion:
1:   0 == 62
andreasbaumann commented 2 years ago

The problem is that the IntIntTable just uses 'int', which is platform-dependent, maybe things get better with int64_t.. (didn't try yet)

manugoyal commented 2 years ago

Hi @andreasbaumann, I think this may be the same issue as https://github.com/efficient/libcuckoo/issues/137, which should be fixed by https://github.com/efficient/libcuckoo/commit/c749c88864d286c3d875b1de2a082355b7838af7. Could you give the test another try on the latest develop?

andreasbaumann commented 2 years ago

Yes, sorry. Didn't see that. I retested 784d0f5d147b9a73f897ae55f6c3712d9a91b058 on Archlinux on IA-32, armv6, armv7 and aarch64. All tests pass.

i486 needs an additional -DCMAKE_CXX_STANDARD_LIBRARIES=-latomic but this can be done outside of the package, no need for support for that inside libcuckoo itself.