From 4c67b4458db0859daf09a29367060c51694dbbb0 Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 09:13:33 +0200 Subject: [PATCH] fix: union/xor of different-sized bitmaps drops bits in variadic path Or and Xor with extra bitmaps used _or_many/_xor_many, which apply a single word count to every input. The count came from pointersOf, whose length is the maximum over the extra slice only and ignores other, so a longer other lost its tail words. More generally a uniform count cannot combine operands of different lengths: a longer source is truncated and a shorter one is read past its end. Fold each source in with the single-operand SIMD primitive over its own length, matching the scalar reference in simd_generic.go. Fixes the amd64 accelerated and AVX-512 branches and the arm64 branch. --- bitmap_amd64.go | 44 ++++++++++++++++++++++++++++++++++++-------- bitmap_arm64.go | 24 ++++++++++++++++++++---- bitmap_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 12 deletions(-) diff --git a/bitmap_amd64.go b/bitmap_amd64.go index 1573905..11ede08 100644 --- a/bitmap_amd64.go +++ b/bitmap_amd64.go @@ -83,16 +83,30 @@ func (dst *Bitmap) Or(other Bitmap, extra ...Bitmap) { case 0: _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _or_many(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + // _or_many uses one word count for every input, so it cannot union + // bitmaps of different lengths. Union each source over its own. + if len(other) > 0 { + _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } case isAVX512: switch len(extra) { case 0: _or_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _or_many_avx512(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + if len(other) > 0 { + _or_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _or_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } default: or(*dst, other, extra) @@ -113,16 +127,30 @@ func (dst *Bitmap) Xor(other Bitmap, extra ...Bitmap) { case 0: _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _xor_many(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + // _xor_many uses one word count for every input, so it cannot combine + // bitmaps of different lengths. Fold in each source over its own. + if len(other) > 0 { + _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } case isAVX512: switch len(extra) { case 0: _xor_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _xor_many_avx512(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + if len(other) > 0 { + _xor_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _xor_avx512(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } default: xor(*dst, other, extra) diff --git a/bitmap_arm64.go b/bitmap_arm64.go index 694c52d..b2a3199 100644 --- a/bitmap_arm64.go +++ b/bitmap_arm64.go @@ -67,8 +67,16 @@ func (dst *Bitmap) Or(other Bitmap, extra ...Bitmap) { case 0: _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _or_many(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + // _or_many uses one word count for every input, so it cannot union + // bitmaps of different lengths. Union each source over its own. + if len(other) > 0 { + _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _or(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } default: or(*dst, other, extra) @@ -89,8 +97,16 @@ func (dst *Bitmap) Xor(other Bitmap, extra ...Bitmap) { case 0: _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) default: - vx, max := pointersOf(other, extra) - _xor_many(unsafe.Pointer(&(*dst)[0]), vx, dimensionsOf(max, len(extra)+1)) + // _xor_many uses one word count for every input, so it cannot combine + // bitmaps of different lengths. Fold in each source over its own. + if len(other) > 0 { + _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&other[0]), uint64(len(other))) + } + for i := range extra { + if len(extra[i]) > 0 { + _xor(unsafe.Pointer(&(*dst)[0]), unsafe.Pointer(&extra[i][0]), uint64(len(extra[i]))) + } + } } default: xor(*dst, other, extra) diff --git a/bitmap_test.go b/bitmap_test.go index 14ab9db..5a16d36 100644 --- a/bitmap_test.go +++ b/bitmap_test.go @@ -629,3 +629,49 @@ func testTruthTables(t *testing.T) { assert.Equal(t, 0b0110, int(a[0])) } } + +func TestOr_ExtraDifferentBitmapSizes(t *testing.T) { + // Ragged inputs: every set bit must survive regardless of which is longest. + var dst Bitmap + dst.Set(1) // word 0 + dst.Set(70) // word 1 + + var other Bitmap + other.Set(3) // word 0 + other.Set(200) // word 3, beyond every extra bitmap + + var short Bitmap + short.Set(5) // word 0 only + + var mid Bitmap + mid.Set(130) // word 2 + + dst.Or(other, short, mid) + + want := []uint32{1, 70, 3, 200, 5, 130} + for _, v := range want { + assert.True(t, dst.Contains(v), "missing bit "+strconv.Itoa(int(v))) + } + assert.Equal(t, len(want), dst.Count()) +} + +func TestXor_ExtraDifferentBitmapSizes(t *testing.T) { + var dst Bitmap + dst.Set(1) + dst.Set(200) // shared with other, must cancel out + + var other Bitmap + other.Set(200) // cancels dst's bit 200 + other.Set(201) // survives, beyond every extra bitmap + + var short Bitmap + short.Set(5) + + dst.Xor(other, short) + + assert.True(t, dst.Contains(1)) + assert.False(t, dst.Contains(200), "bit 200 should cancel") + assert.True(t, dst.Contains(201), "bit 201 from longer source must survive") + assert.True(t, dst.Contains(5)) + assert.Equal(t, 3, dst.Count()) +}