modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo/manual/
Other
23.13k stars 2.59k forks source link

[BUG] `DTypePointer[bool]` packs bits inconsistently #2813

Closed helehex closed 4 months ago

helehex commented 5 months ago

Bug description

When using DTypePointer[bool] store()/load() with different widths, you get inconsistent results.

Steps to reproduce

var ptr = DTypePointer[DType.bool].alloc(4)
ptr.store(0, True)
ptr.store(1, True)
ptr.store(2, True)
ptr.store(3, True)
print(ptr.load[width=4](0))

prints: [True, False, False, False]

System information

Host Information
  ================

  Target Triple: x86_64-unknown-linux
  CPU: skylake
  CPU Features: adx, aes, avx, avx2, bmi, bmi2, clflushopt, cmov, crc32, cx16, cx8, f16c, fma, fsgsbase, fxsr, invpcid, lzcnt, mmx, movbe, pclmul, popcnt, prfchw, rdrnd, rdseed, sahf, sgx, sse, sse2, sse3, sse4.1, sse4.2, ssse3, x87, xsave, xsavec, xsaveopt, xsaves

mojo 2024.5.2405 (fc1ed0a3)

modular 0.8.0 (39a426b5)
MadAlex1997 commented 5 months ago

Probably Related to 2492, I am pretty sure there are issues at the DType pointer level with the i1 representation of SIMD[DType.bool,1] and the 8 Bit C style bool used in the pointers.

bgreni commented 4 months ago

Seems as though this has been fixed with recent changes?

var ptr = DTypePointer[DType.bool].alloc(4)
SIMD[size=4].store(ptr, SIMD[DType.bool, 4](True, False, True, True))
print(SIMD[size=4].load(ptr)) # prints [True, False, True, True]
rparolin commented 4 months ago

Looks like the patch mentioned above by @bgreni resolved the issue. I'll close this issue but we can reopen it if knock-ons are discovered. Thank you!

helehex commented 4 months ago

looks like when store/load moved to SIMD, the fix wasn't

helehex commented 4 months ago
var ptr = DTypePointer[DType.bool].alloc(4)
SIMD[DType.bool, 1].store(ptr, 0, True)
SIMD[DType.bool, 1].store(ptr, 1, True)
SIMD[DType.bool, 1].store(ptr, 2, True)
SIMD[DType.bool, 1].store(ptr, 3, True)
print(SIMD[DType.bool, 4].load(ptr, 0))

still prints [True, False, False, False]

helehex commented 4 months ago

The inconsistency is that width packs bits, but the offset is in bytes. This means everything gets weird when using different widths on the same pointer.

SIMD[DType.bool, 1].store(ptr, 0, True)
SIMD[DType.bool, 1].store(ptr, 1, True)
# b00000001
# b00000001

vs:

SIMD[DType.bool, 2].store(ptr, 0, True)
# b00000011
bgreni commented 4 months ago

@helehex hmm yeah I see your point, loading the entire thing in/out works better now, but trying to do piecemeal operations still doesn't work.

MarkOWiesemann commented 3 months ago

Is this supposed to work now? I am on mojo 24.4.0 (2cb57382)

MadAlex1997 commented 3 months ago

Is this supposed to work now? I am on mojo 24.4.0 (2cb57382)

They fixed it in nightly but it did not make it into 24.4. And I don't know if it got reverted in nightly.

def main():
    var boolpointer = DTypePointer[DType.bool]().alloc(32)
    boolpointer.store[width=64](0,SIMD[DType.bool,1](True))
    print(boolpointer)
    for i in range(32):
        print(boolpointer[i])
True
True
True
True
True
True
True
True
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False
False