EdB <edb+m...@sigluy.net> writes: Looks OK to me, some nitpicks related to the coding style below.
> ---
> src/gallium/state_trackers/clover/api/dispatch.cpp | 4 +-
> src/gallium/state_trackers/clover/api/event.cpp | 48
> ++++++++++++++++++----
> 2 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/dispatch.cpp
> b/src/gallium/state_trackers/clover/api/dispatch.cpp
> index 746372c..e4f7ea3 100644
> --- a/src/gallium/state_trackers/clover/api/dispatch.cpp
> +++ b/src/gallium/state_trackers/clover/api/dispatch.cpp
> @@ -129,8 +129,8 @@ namespace clover {
> NULL, // clEnqueueFillBuffer
> NULL, // clEnqueueFillImage
> NULL, // clEnqueueMigrateMemObjects
> - NULL, // clEnqueueMarkerWithWaitList
> - NULL, // clEnqueueBarrierWithWaitList
> + clEnqueueMarkerWithWaitList,
> + clEnqueueBarrierWithWaitList,
> NULL, // clGetExtensionFunctionAddressForPlatform
> NULL, // clCreateFromGLTexture
> NULL, // clGetDeviceIDsFromD3D11KHR
> diff --git a/src/gallium/state_trackers/clover/api/event.cpp
> b/src/gallium/state_trackers/clover/api/event.cpp
> index 6b1956c..daf9577 100644
> --- a/src/gallium/state_trackers/clover/api/event.cpp
> +++ b/src/gallium/state_trackers/clover/api/event.cpp
> @@ -179,6 +179,27 @@ clEnqueueMarker(cl_command_queue d_q, cl_event *rd_ev)
> try {
> }
>
> CLOVER_API cl_int
> +clEnqueueMarkerWithWaitList(cl_command_queue d_q, cl_uint num_evs,
> + const cl_event *d_evs, cl_event *rd_ev) try {
> + auto &q = obj(d_q);
> + auto deps = objs<wait_list_tag>(d_evs, num_evs);
> +
> + for (auto &ev : deps) {
The indentation is weird here.
> + if (ev.context() != q.context())
> + throw error(CL_INVALID_CONTEXT);
> + }
> +
> + // Create a hard event that depends on the events in the wait list.
I think it would be useful to mention here that this event is implicitly
serialized with respect to the previous events in the command queue.
> + auto hev = create<hard_event>(q, CL_COMMAND_MARKER, deps);
> +
> + ret_object(rd_ev, hev);
> + return CL_SUCCESS;
> +
> +} catch (error &e) {
> + return e.get();
> +}
> +
> +CLOVER_API cl_int
> clEnqueueBarrier(cl_command_queue d_q) try {
> obj(d_q);
>
> @@ -191,21 +212,20 @@ clEnqueueBarrier(cl_command_queue d_q) try {
> }
>
> CLOVER_API cl_int
> -clEnqueueWaitForEvents(cl_command_queue d_q, cl_uint num_evs,
> - const cl_event *d_evs) try {
> +clEnqueueBarrierWithWaitList(cl_command_queue d_q, cl_uint num_evs,
> + const cl_event *d_evs, cl_event *rd_ev) try {
> auto &q = obj(d_q);
> - auto evs = objs(d_evs, num_evs);
> + auto deps = objs<wait_list_tag>(d_evs, num_evs);
The event list was named "evs" after its corresponding API objects
d_evs, and num_evs. It would be nice if you kept that correspondence,
e.g. by renaming the entry-point arguments to d_deps and num_deps. The
same applies to clEnqueueMarkerWithWaitList().
>
> - for (auto &ev : evs) {
> + for (auto &ev : deps) {
Odd indentation again.
> if (ev.context() != q.context())
> throw error(CL_INVALID_CONTEXT);
> }
>
> - // Create a hard event that depends on the events in the wait list:
> - // subsequent commands in the same queue will be implicitly
> - // serialized with respect to it -- hard events always are.
> - create<hard_event>(q, 0, evs);
> + // Create a hard event that depends on the events in the wait list.
I think the comment was more informative the way it was.
> + auto hev = create<hard_event>(q, CL_COMMAND_BARRIER, deps);
>
> + ret_object(rd_ev, hev);
> return CL_SUCCESS;
>
> } catch (error &e) {
> @@ -213,6 +233,18 @@ clEnqueueWaitForEvents(cl_command_queue d_q, cl_uint
> num_evs,
> }
>
> CLOVER_API cl_int
> +clEnqueueWaitForEvents(cl_command_queue d_q, cl_uint num_evs,
> + const cl_event *d_evs) try {
> + // The wait list is mandatory for clEnqueueWaitForEvents().
> + objs(d_evs, num_evs);
> +
> + return clEnqueueBarrierWithWaitList(d_q, num_evs, d_evs, NULL);
> +
> +} catch (error &e) {
> + return e.get();
> +}
> +
> +CLOVER_API cl_int
> clGetEventProfilingInfo(cl_event d_ev, cl_profiling_info param,
> size_t size, void *r_buf, size_t *r_size) try {
> property_buffer buf { r_buf, size, r_size };
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgp12eQ96z7uS.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
