Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

64-bit capable containers #17

Open
kknox opened this issue Apr 26, 2015 · 6 comments
Open

64-bit capable containers #17

kknox opened this issue Apr 26, 2015 · 6 comments

Comments

@kknox
Copy link
Contributor

kknox commented Apr 26, 2015

Currently, all of our C-based structs store their dimension sizes as cl_int. This limits the sizes of the sparse matrices our library supports to 2-billion (this was chosen without much thought at the time). Should we possibly redefine the structs to include all sizes as cl_ulong, or possible size_t? Size_t may be problematic, in that OpenCL does not allow size_t types as kernel arguments
From spec:
Arguments to __kernel functions in a program cannot be declared with the built-in scalar types bool, half, size_t, ptrdiff_t, intptr_t, and uintptr_t

@jpola
Copy link
Contributor

jpola commented Apr 27, 2015

I vote for cl_ulong.

@kknox
Copy link
Contributor Author

kknox commented Apr 27, 2015

I also lean in this direction; it would be nice to think we could handle 64bit sizes, but what would be the occurance of this in the 'real world'? Is it enough to just change cl_int to cl_uint? 64-bit data/registers can reduce kernel performance.

@jpola
Copy link
Contributor

jpola commented Apr 27, 2015

If we change that we need to check if the kernel wrapper is receiving correct data type and probably the kernel parameters string need to be changed accordingly. kernel parameter string defines the types of the kernel parameters by substituting #define variables in the kernel code.

I will take care of it.

@jpola jpola self-assigned this Apr 27, 2015
@kknox
Copy link
Contributor Author

kknox commented Apr 29, 2015

The decision of whether we should switch our structures to use 64-bit members is somewhat dependent on how this affects kernel performance. I talk about this some in this comment: #20 (comment)

This idea is dependent on the new benchmark framework proposed in #21. We need to be able to measure performance of this change to determine if this is appropriate.

@kknox kknox assigned kknox and unassigned jpola May 29, 2015
@kknox
Copy link
Contributor Author

kknox commented May 29, 2015

I've going to take care of this as I work on SpM-dM.

I've been looking at the OpenCL API, and all the API's that take size parameters are taking size_t's. I think that is a good standard, and I am going to work on changing our data structures to use size_t's. We have to be careful not to copy size_t's to clSetKernelArg(), because the device may have a different address depth than host. We should dynamically detect the address depth of the device and cast the values we pass into clSetKernelArg() and equivalents.

@jpola
Copy link
Contributor

jpola commented May 29, 2015

From my other projects we were taking cl_ulong for sizes. Be careful with the matrices, type for indexes are important.

@kknox kknox added this to the clSPARSE Beta milestone Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants