CL_MEM_USE_HOST_PTR for CLArrays instead of CL_MEM_ALLOC_HOST_PTR for…

[lixfel]

… alignment reasons

[guyfleeman]

The doc in the comment says the buffer needs to be page aligned and a multiple of the L1 cache line size. Not sure if it's worth documenting that aligned_alloc will only ever allocate a buffer that's an integer multiple of the alignment, so the 64 byte or even 128byte L1 cache alignment in arm64 is guaranteed. I think it's sound.

I had to look this up because I thought it could be wrong based on the C/C++ aligned intrinsic. For example

struct __attribute__((aligned(4096))) MyStruct {
    char data[2000]; 
}; 

would NOT be valid to set the CL_MEM_USE_HOST_PTR bit. This ptr would be both 4096 (and thus 1024 for Spinnaker) byte aligned, but is not a multiple of the L1 DCACHE size on x64 or arm64.

This is all just to say, I did spend some time critically analyzing the proposed fix and I think it's good.

sysconf(_SC_PAGESIZE);

Is the programmatic way to get page size, but Ubuntu has a compatibility layer for apps/DMA even when huge pages are on (which they are not by default). Since that's our target platform, hard coding is fine.

[lixfel]

would NOT be valid to set the CL_MEM_USE_HOST_PTR bit. This ptr would be both 4096 (and thus 1024 for Spinnaker) byte aligned, but is not a multiple of the L1 DCACHE size on x64 or arm64.

I don't think there will ever be a cache line which will be larger than the page size or not hove a power of two size. Under this assumption the 4 KiB alignment will always also lead to cache line alignment. The bigger issue in my opinion is the page size of 64 KiB used by the Apple M* architecture.

[guyfleeman]

Yeah agreed about the M* concerns, and general arm64 stuff going forward. I wouldn't be surprised if a future gen Nvidia Orin type thing also widened to 128bytes like Apple.

[lixfel]

I'm not concerned about cache line size here, as it's a reasonable assumption (imho) that the page size alignment will always guarantee cache line alignment.

[guyfleeman]

Just FYI, I only have Spinnaker GigE cameras, not USB. This looks solid in principle, but can't test it for real.