Skip to content

CL_MEM_USE_HOST_PTR for CLArrays instead of CL_MEM_ALLOC_HOST_PTR for… - #25

Open
lixfel wants to merge 1 commit into
masterfrom
fix-spinnaker
Open

CL_MEM_USE_HOST_PTR for CLArrays instead of CL_MEM_ALLOC_HOST_PTR for…#25
lixfel wants to merge 1 commit into
masterfrom
fix-spinnaker

Conversation

@lixfel

@lixfel lixfel commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

… alignment reasons

@guyfleeman

Copy link
Copy Markdown

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

lixfel commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants