[Mesa-dev] [PATCH 0/3] cl workdim v2

[Jan Vesely]

On Thu, 2014-08-07 at 16:02 +0300, Francisco Jerez wrote:
> Jan Vesely <jan.vesely at rutgers.edu> writes:
> 
> > This respin includes Francisco's approach of providing implicit
> > in the arg vector passed from clover, and Tom's idea of appending
> > implicit args after the kernel args.
> >
> 
> Hmmm...  Maybe it would make sense to add some sort of versioning
> (e.g. as part of the target triple) to the binary interface between
> clover and the kernel instead, so we can handle this sort of
> non-backwards compatible changes and the compiler back-end and libclc
> have some way to find out whether some specific feature is available and
> e.g. some specific extension should be enabled.
> 
> > I assumed it's not safe to modify exec.input, so the input vector is copied
> > before appending work dim.
> >
> 
> Why wouldn't it be safe?  You just need to make sure they're appended
> before the compute state is created.

I thought there might be a problem when called from multiple threads,
but it looks like most of the vars are local to the current call anyway.

I looked at the code a bit better, and need a bit of help with what the
proffered approach would be.

exec_context::bind() appends all kernel args to the input vector. If the
implicit args are added before bind() it shifts all other args, which is
not what we want.
if the implicit args are appended after, they are not accounted for in
shader->input_size (and not copied by the driver).

my current code modifies exec_context::bind() to preserve the content of
input before binding kernel args, and append the old content after the
args are bound.
I have also considered passing and implicit args vector to
exec_context::bind to make the trick more visible.

Turning workdim into a proper arg in _args does not work either, because
it is not present in module args.

any thoughts?

thanks,
jan


> 
> > Passes get-work-dim piglit on turks without any regression,
> > I have not tested SI as I don't have the hw.
> >
> > jan
> >
> >
> >
> >
> > Jan Vesely (3):
> >   gallium: Pass input data size to launch_grid
> >   clover: Add work dimension implicit param to input
> >   r600,radeonsi: Copy implicit args provided by clover
> >
> >  src/gallium/drivers/ilo/ilo_gpgpu.c               |   2 +-
> >  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c   |   2 +-
> >  src/gallium/drivers/nouveau/nvc0/nvc0_context.h   |   4 +-
> >  src/gallium/drivers/nouveau/nvc0/nve4_compute.c   |   2 +-
> >  src/gallium/drivers/r600/evergreen_compute.c      |  14 +-
> >  src/gallium/drivers/r600/evergreen_compute.h      |   1 -
> >  src/gallium/drivers/radeonsi/si_compute.c         |   6 +-
> >  src/gallium/include/pipe/p_context.h              |   2 +-
> >  src/gallium/state_trackers/clover/core/kernel.cpp | 162 ++++++++++++----------
> >  src/gallium/tests/trivial/compute.c               |  40 +++---
> >  10 files changed, 122 insertions(+), 113 deletions(-)
> >
> > -- 
> > 1.9.3

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140808/510766aa/attachment.sig>