Skip to content

Commit 24d71b5

Browse files
committed
Address some feedback from rust-lang PR
1 parent 381d683 commit 24d71b5

File tree

4 files changed

+26
-18
lines changed

4 files changed

+26
-18
lines changed

src/raw/generic.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ fn repeat(byte: u8) -> GroupWord {
3636
/// parallel.
3737
///
3838
/// This implementation uses a word-sized integer.
39+
#[derive(Copy, Clone)]
3940
pub struct Group(GroupWord);
4041

4142
// We perform all operations in the native endianess, and convert to
@@ -52,16 +53,14 @@ impl Group {
5253
/// This is guaranteed to be aligned to the group size.
5354
#[inline]
5455
pub fn static_empty() -> &'static [u8] {
55-
#[repr(C)]
56-
struct Dummy {
57-
_align: [GroupWord; 0],
56+
union AlignedBytes {
57+
_align: Group,
5858
bytes: [u8; Group::WIDTH],
5959
};
60-
const DUMMY: Dummy = Dummy {
61-
_align: [],
60+
const ALIGNED_BYTES: AlignedBytes = AlignedBytes {
6261
bytes: [EMPTY; Group::WIDTH],
6362
};
64-
&DUMMY.bytes
63+
unsafe { &ALIGNED_BYTES.bytes }
6564
}
6665

6766
/// Loads a group of bytes starting at the given address.
@@ -71,16 +70,18 @@ impl Group {
7170
}
7271

7372
/// Loads a group of bytes starting at the given address, which must be
74-
/// aligned to `WIDTH`.
73+
/// aligned to `mem::align_of::<Group>()`.
7574
#[inline]
7675
pub unsafe fn load_aligned(ptr: *const u8) -> Group {
76+
debug_assert_eq!(ptr as usize & (mem::align_of::<Group>() - 1), 0);
7777
Group(ptr::read(ptr as *const _))
7878
}
7979

8080
/// Stores the group of bytes to the given address, which must be
81-
/// aligned to `WIDTH`.
81+
/// aligned to `mem::align_of::<Group>()`.
8282
#[inline]
8383
pub unsafe fn store_aligned(&self, ptr: *mut u8) {
84+
debug_assert_eq!(ptr as usize & (mem::align_of::<Group>() - 1), 0);
8485
ptr::write(ptr as *mut _, self.0);
8586
}
8687

src/raw/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ unsafe fn offset_from<T>(to: *const T, from: *const T) -> usize {
3636
}
3737

3838
// Use the SSE2 implementation if possible: it allows us to scan 16 buckets at
39-
// once instead of 8.
39+
// once instead of 8. We don't bother with AVX since it would require runtime
40+
// dispatch and wouldn't gain us much anyways: the probability of finding a
41+
// match drops off drastically after the first few buckets.
42+
//
43+
// I attempted an implementation on ARM using NEON instructions, but it turns
44+
// out that most NEON instructions have multi-cycle latency, which in the end
45+
// outweighs any gains over the generic implementation.
4046
#[cfg(all(
4147
target_feature = "sse2",
4248
any(target_arch = "x86", target_arch = "x86_64")

src/raw/sse2.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub const BITMASK_MASK: u16 = 0xffff;
1515
/// parallel.
1616
///
1717
/// This implementation uses a 128-bit SSE value.
18+
#[derive(Copy, Clone)]
1819
pub struct Group(x86::__m128i);
1920

2021
impl Group {
@@ -27,16 +28,14 @@ impl Group {
2728
/// This is guaranteed to be aligned to the group size.
2829
#[inline]
2930
pub fn static_empty() -> &'static [u8] {
30-
#[repr(C)]
31-
struct Dummy {
32-
_align: [x86::__m128i; 0],
31+
union AlignedBytes {
32+
_align: Group,
3333
bytes: [u8; Group::WIDTH],
3434
};
35-
const DUMMY: Dummy = Dummy {
36-
_align: [],
35+
const ALIGNED_BYTES: AlignedBytes = AlignedBytes {
3736
bytes: [EMPTY; Group::WIDTH],
3837
};
39-
&DUMMY.bytes
38+
unsafe { &ALIGNED_BYTES.bytes }
4039
}
4140

4241
/// Loads a group of bytes starting at the given address.
@@ -46,16 +45,18 @@ impl Group {
4645
}
4746

4847
/// Loads a group of bytes starting at the given address, which must be
49-
/// aligned to `WIDTH`.
48+
/// aligned to `mem::align_of::<Group>()`.
5049
#[inline]
5150
pub unsafe fn load_aligned(ptr: *const u8) -> Group {
51+
debug_assert_eq!(ptr as usize & (mem::align_of::<Group>() - 1), 0);
5252
Group(x86::_mm_load_si128(ptr as *const _))
5353
}
5454

5555
/// Stores the group of bytes to the given address, which must be
56-
/// aligned to `WIDTH`.
56+
/// aligned to `mem::align_of::<Group>()`.
5757
#[inline]
5858
pub unsafe fn store_aligned(&self, ptr: *mut u8) {
59+
debug_assert_eq!(ptr as usize & (mem::align_of::<Group>() - 1), 0);
5960
x86::_mm_store_si128(ptr as *mut _, self.0);
6061
}
6162

tests/set.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ extern crate hashbrown;
22
extern crate rand;
33

44
use hashbrown::HashSet;
5-
use rand::{Rng, SeedableRng, XorShiftRng, distributions::Alphanumeric};
5+
use rand::{distributions::Alphanumeric, Rng, SeedableRng, XorShiftRng};
66

77
#[test]
88
fn test_hashset_insert_remove() {

0 commit comments

Comments
 (0)