search

clMathLibraries / clSPARSE

a software library containing Sparse functions written in OpenCL
Apache License 2.0
173 stars 60 forks source link

Fix the kernel wrapper for cl2.hpp. #199

Closed jlgreathouse closed 7 years ago

jlgreathouse commented 7 years ago

We must pass the address of the cl_mem argument, not the cl_mem itself.

Without this patch, we currently segfault when trying to run kernels that use a cl_mem argument. We essentially end up passing garbage into clSetKernelArg()'s argument variable.

Tested on test-blas2 and test-blas3 with Linux AMD APP SDK 3.0 and fglrx drivers.

dagamayank commented 7 years ago

How was anything even working before then?

jlgreathouse commented 7 years ago

I don't know if anyone's run any functional tests (rather than just build tests) since the June 30 update which moved us from cl.hpp to cl2.hpp.

I believe this change switched which sestArg() function was called.

The two-argument setArg() function automatically calculates the pointer to the value you pass it, internally. The three-argument setArg() spects you to pass the pointer yourself.

jlgreathouse commented 7 years ago

Note that all the checks fail for this commit because the testing tools cannot build clSPARSE right now. clSPARSE currently relies on gmock-1.7.0 being hosted on googlemock.googlecode.com.

Google shut down googlecode.com hosting a few months ago. I've already sent this request to Kent to fix. I have a quick workaround running on my machine (changing cmake/ExternalGmock.cmake line 28 to point to this other host for that file). Kent is working on a more robust, full solution for this build problem.

I'd still request this be merged. Perhaps someone else could manually test it -- but the build problems above are not because of my change.