storableLaws/storablePeekByte shouldn't expect every element of an array via `newArray` to be aligned?

[mikeplus64]

https://github.com/hedgehogqa/haskell-hedgehog-classes/blob/69212627dca7b21252a55495299e9bb84434d2ac/src/Hedgehog/Classes/Storable.hs#L76-L87

In the implementation of storablePeekByte, every element's offset into the array is aligned to the Storable instance's supplied alignment.

However, this is not how GHC actually writes Storable arrays. That's how I thought alignment would be treated as well -- that each element of an array would be aligned. That does not seem to be the case, at least in GHC functions like newArray, pokeArray, etc. -- see https://gitlab.haskell.org/ghc/ghc/-/blob/dbd852f5dc641ae4f41ac242e394f9b68587e701/libraries/ghc-internal/src/GHC/Internal/Foreign/Marshal/Array.hs#L176 -- they all seem to write array elements as compactly as possible. Functions like allocaArray use alignment to align the address of the array itself but say nothing of the elements.

For comparison, quickcheck-classes implementation of the storablePeekByte property does not align every element offset.

https://github.com/andrewthad/quickcheck-classes/blob/ecd8ceb1c30a23d7edbe4af552bdc4be6274b773/quickcheck-classes-base/src/Test/QuickCheck/Classes/Storable.hs#L62-L70

[andrewthad]

I'm not sure that I understand your claim. Here's the implementation of pokeArray that you link to:

pokeArray :: Storable a => Ptr a -> [a] -> IO ()
{-# INLINEABLE pokeArray #-}
pokeArray ptr vals0 = go vals0 0#
  where go [] _          = return ()
        go (val:vals) n# = do pokeElemOff ptr (I# n#) val; go vals (n# +# 1#)

This uses pokeElemOff, which scales the offset by the element size. If the ptr argument is aligned in a way that agrees with the Storable instances, all of the elements that are written to the array will be aligned. The function doesn't specifically say "make sure the pointer is aligned correctly" as a precondition, but I've always assumed that GHC (or at least the Haskell2010 subset of it) intends for pointers to be aligned.

Also, newArray calls mallocArray which calls mallocBytes, so you'll get a pointer that 8-byte aligned. I suspect that anything relying on pointers with 16-byte or 32-byte alignment is probably not actually going to get what it is hoping for, but that's more of an issue with GHC.

[mikeplus64]

Yeah OK, makes sense.

Rephrasing to sanity check myself: If all pointers must be aligned by an alignment, then you must also have the property that ptr+n*sizeOf ptr === alignPtr (ptr+(n*sizeOf ptr)). So size must be divisible by alignment. If I do want a compact Storable, then I can use e.g. alignment _ = 1. Fitting more into cache could be more valuable to performance than proper alignment in some cases.

If a Storable must have that property, then I think it would be good to introduce a separate test for it (that size is divisible by alignment). That makes sense to me because yeah, what good is it to only align the first element of an array, anyway?

pokeElemOff itself will not call alignPtr - with the default definition pokeElemOf ptr off val = pokeByteOff ptr (off * sizeOf val) val = poke (plusPtr ptr off) val - so instances with incorrect size/alignment will have arrays from newArray et al written more compactly than they should be.