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.
This commit is contained in:
1 parent
831b48a3dd
commit
4c67b4458d
3 files changed
+102
-12
No files matched your search
+36
-8
@@ -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)
|
||||
|
||||
+20
-4
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
Reference in new issue
Block a user