search

luqmana / rust-opencl

OpenCL bindings for Rust.
Other
170 stars 42 forks source link

Major refactoring #19

Closed ghost closed 11 years ago

ghost commented 11 years ago

The goal of this pull request is refactoring two sections. One is the removal of the ComputeContext, since it had a tendency to be used everywhere and people would just peak into it. The reason for this was to give the API user more direct control over which queues were doing the work, and how things were synchronized. Abstracting the read() and writes() for example in the vector or unique class forces the api user to wait before fetching data.

The command queue, the device, and the context have all been flushed out with some of the functionality that was only reserved for the command queue at one time.

Since it is pretty annoying to initialize opencl without the ComputeContext, a utility function is added that gives the user a context, device, and command queue.

The memory subsystem needed to be rework to. My goal was to allow the user to define how to read and write data into the opencl runtime allowing for them to do simple transfers like a vector read or write. Or more complicated things like reading the header to determine the size of the object before reading it into the host's memory space.

Buffer is also rework to allow for user implementations. And they are now typed to avoid unsafe conditions and better type inference.

There is an addition of a 2D/3D array type, which was one of the motivations for the rework to begin with.

eholk commented 11 years ago

Hey, sorry I didn't get the chance to give you any feedback earlier. Are you still interested in doing this refactoring?

ghost commented 11 years ago

Yeah I just want to cleanup the history, I did a few mergers back to trunk and the commit history does not look right.

itdaniher commented 11 years ago

@csherratt I built your master branch earlier today and poked around. Much cleaner memory handling and a few other nice additions. Looking forward to seeing it merged!

eholk commented 11 years ago

This looks greatly improved!

I was working to merge these, but unfortunately I got a segmentation fault from Travis CI. Do you have any idea what might be causing it?

https://travis-ci.org/eholk/rust-opencl/builds/13409549

eholk commented 11 years ago

I got 6 test failures running under valgrind as well. That might be a bit easier to debug than pushing changes out to Travis.

ghost commented 11 years ago

I'll take a look. I have tested it with Intel, Nvidia & Apple implementation I think it might have something to do with ATIs implementation.

ghost commented 11 years ago

turned out the AMD compiler was crashing when it compiled the kernel.

struct vec { 
    long fill;
    long alloc;
    long dat[]; // Crashes AMD OpenCL Compiler.
};

Unsure of the valgrind errors running valgrind locally with the AMD OpenCL implementation segfaults before it completes. Even on the current master.

https://travis-ci.org/csherratt/rust-opencl/builds/13420813

eholk commented 11 years ago

Thanks!

Most of the stacks I saw in valgrind were from the OpenCL library itself, so I'm not as worried about those.