Jan Vesely <[email protected]> writes: > Specs say it's legal. > Fixes clones.xml gegl test. > > --- > Hi, > > this patch is an attempt to support NULL buffer objects in clover. > It adds NULL handling to few places, and it's enough to get 'clones' > gegl test running. > > The specs say: "If the argument is a buffer object, the arg_value > pointer can be NULL or point to a NULL value in which case a NULL > value will be used as the value for the argument declared as a > pointer to __global or __constant memory in the kernel." > > I was considering using a special buffer instance to represent > NULL buffers but this seems more straightforward. Using 'pobj' > should take care of the latter part of the spec requirement too. > > BZ: https://bugs.freedesktop.org/show_bug.cgi?id=73571#c2 > ignore the first two comments I forgot I was testing some > additional patches. > > Tom, I'm not sure I understand your comment. Does it mean that at > the moment it's possible that kernels receive NULL pointers > that point to valid buffers? > > regards, > Jan > > src/gallium/drivers/r600/evergreen_compute.c | 9 ++++++--- > src/gallium/state_trackers/clover/core/kernel.cpp | 4 ++-- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/drivers/r600/evergreen_compute.c > b/src/gallium/drivers/r600/evergreen_compute.c > index a2db69b..f14b7c5 100644 > --- a/src/gallium/drivers/r600/evergreen_compute.c > +++ b/src/gallium/drivers/r600/evergreen_compute.c > @@ -655,10 +655,13 @@ static void evergreen_set_global_binding( > > for (int i = 0; i < n; i++) > { > - assert(resources[i]->target == PIPE_BUFFER); > - assert(resources[i]->bind & PIPE_BIND_GLOBAL); > + if (resources[i]) { > + assert(resources[i]->target == PIPE_BUFFER); > + assert(resources[i]->bind & PIPE_BIND_GLOBAL); > > - *(handles[i]) = buffers[i]->chunk->start_in_dw * 4; > + *(handles[i]) = buffers[i]->chunk->start_in_dw * 4; > + } else > + *(handles[i]) = NULL; > } > > evergreen_set_rat(ctx->cs_shader_state.shader, 0, pool->bo, 0, > pool->size_in_dw * 4);
Can you split the r600 changes into a separate patch?
> diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp
> b/src/gallium/state_trackers/clover/core/kernel.cpp
> index ac57c71..412eac4 100644
> --- a/src/gallium/state_trackers/clover/core/kernel.cpp
> +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
> @@ -331,7 +331,7 @@ kernel::global_argument::set(size_t size, const void
> *value) {
> if (size != sizeof(cl_mem))
> throw error(CL_INVALID_ARG_SIZE);
>
> - buf = &obj<buffer>(*(cl_mem *)value);
> + buf = pobj<buffer>(value ? *(cl_mem *)value : NULL);
> _set = true;
> }
>
> @@ -340,7 +340,7 @@ kernel::global_argument::bind(exec_context &ctx,
> const module::argument &marg) {
> align(ctx.input, marg.target_align);
> ctx.g_handles.push_back(allocate(ctx.input, marg.target_size));
> - ctx.g_buffers.push_back(buf->resource(*ctx.q).pipe);
> + ctx.g_buffers.push_back(buf ? buf->resource(*ctx.q).pipe: NULL);
> }
>
Another possibility would be to write NULL to the input buffer here and
not push the handle/buffer pair at all. That way drivers won't have to
deal with this special case.
You probably need to fix constant buffer arguments too.
Thanks.
> void
> --
> 1.8.4.2
pgpAVG7d2AZhj.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
