[Piglit] [PATCH 3/3] arb_shader_image_load_store: call glMemoryBarrier() before reading an image written to by imageStore()

Timothy Arceri timothy.arceri at collabora.com
Sat Dec 12 20:02:46 PST 2015 

On Fri, 2015-12-11 at 16:05 +0200, Francisco Jerez wrote:
> Timothy Arceri <timothy.arceri at collabora.com> writes:
> 
> > On Thu, 2015-12-10 at 16:21 +0200, Francisco Jerez wrote:
> > > Timothy Arceri <timothy.arceri at collabora.com> writes:
> > > 
> > > > I haven't been able to test if this fixes the bug as I cannot
> > > > reproduce it.
> > > > 
> > > > Cc: Francisco Jerez <currojerez at riseup.net>
> > > > Cc: Jordan Justen <jordan.l.justen at intel.com>
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=92822
> > > > ---
> > > >  .../image_store/basic-imageStore-const-uniform
> > > > -index.shader_test  
> > > >     | 2 ++
> > > >  .../basic-imageStore-mixed-const-non-const-uniform
> > > > -index.shader_test  | 2 ++
> > > >  .../basic-imageStore-mixed-const-non-const-uniform
> > > > -index2.shader_test | 2 ++
> > > >  .../image_store/basic-imageStore-non-const-uniform
> > > > -index.shader_test  | 4 ++++
> > > >  .../execution/basic-imageStore-from-uniform.shader_test       
> > > >     
> > > >     | 2 ++
> > > 
> > > There seem to be a bunch more shader_runner test cases where
> > > we're
> > > missing barriers according to 'git grep -l "image texture "'. 
> > >  The CS
> > > cases currently rely on the hack in shader_runner.c:2778 and
> > > :2780
> > > which
> > > would be nice to clean up at some point, but I guess that could
> > > be
> > > done
> > > as a separate series if you don't feel like doing it now. 
> > 
> > Yeah I noticed the CS hack. I'd rather land this change first and
> > fix
> > up the other cases when I have some free time, or someone else can
> > do
> > it if they are interested.
> 
> Sounds reasonable to me, 

I've pushed v1 of the series to resolve reported bug.

> but I think you've missed several cases that
> don't use compute shaders so the hack won't help (have a look at the
> output from git-grep), e.g.:
> 

These seem to be the only two I've mised right?

>  tests/spec/arb_shader_image_load_store/execution/gl45
> -imageAtomicExchange-float.shader_test

I've sent a follow up patch for this one.

>  tests/spec/arb_shader_image_load_store/execution/load-from-cleared
> -image.shader_test

The image is read only here so there should be no requirement for
memory barrier.


> 
> > 
> > >  Other than
> > > that the general approach seems reasonable to me.
> > > 
> > > >  5 files changed, 12 insertions(+)
> > > > 
> > > > diff --git
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-const-uniform-index.shader_test
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-const-uniform-index.shader_test
> > > > index af7d5d4..ed34b39 100644
> > > > ---
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-const-uniform-index.shader_test
> > > > +++
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-const-uniform-index.shader_test
> > > > @@ -45,6 +45,7 @@ fb tex 2d 1
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 1.0 0.0 0.0 1.0
> > > >  
> > > > @@ -54,5 +55,6 @@ fb tex 2d 1
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 0.0 1.0 0.0 1.0
> > > > diff --git
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index.shader_test
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index.shader_test
> > > > index 519ae0b..ebaeb5d 100644
> > > > ---
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index.shader_test
> > > > +++
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index.shader_test
> > > > @@ -52,6 +52,7 @@ fb tex 2d 2
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 0
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 1.0 0.0 0.0 1.0
> > > >  
> > > > @@ -62,5 +63,6 @@ fb tex 2d 2
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 1
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 1
> > > >  probe all rgba 0.0 1.0 0.0 1.0
> > > > diff --git
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index2.shader_test
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index2.shader_test
> > > > index 1d33d3f..6bd1467 100644
> > > > ---
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index2.shader_test
> > > > +++
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-mixed-const-non-const-uniform-index2.shader_test
> > > > @@ -52,6 +52,7 @@ fb tex 2d 2
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 0
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 1.0 0.0 0.0 1.0
> > > >  
> > > > @@ -62,5 +63,6 @@ fb tex 2d 2
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 1
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 1
> > > >  probe all rgba 0.0 1.0 0.0 1.0
> > > > diff --git
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-non-const-uniform-index.shader_test
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-non-const-uniform-index.shader_test
> > > > index 646ea3b..62f9861 100644
> > > > ---
> > > > a/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-non-const-uniform-index.shader_test
> > > > +++
> > > > b/tests/spec/arb_arrays_of_arrays/execution/image_store/basic
> > > > -imageStore-non-const-uniform-index.shader_test
> > > > @@ -64,6 +64,7 @@ fb tex 2d 4
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 0
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 1.0 0.0 0.0 1.0
> > > >  
> > > > @@ -75,6 +76,7 @@ fb tex 2d 4
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 1
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 1
> > > >  probe all rgba 0.0 1.0 0.0 1.0
> > > >  
> > > > @@ -86,6 +88,7 @@ fb tex 2d 4
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 2
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 2
> > > >  probe all rgba 0.0 0.0 1.0 1.0
> > > >  
> > > > @@ -97,5 +100,6 @@ fb tex 2d 4
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore 3
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 3
> > > >  probe all rgba 0.0 1.0 1.0 1.0
> > > > diff --git
> > > > a/tests/spec/arb_shader_image_load_store/execution/basic
> > > > -imageStore-from-uniform.shader_test
> > > > b/tests/spec/arb_shader_image_load_store/execution/basic
> > > > -imageStore
> > > > -from-uniform.shader_test
> > > > index 7133593..f3e1084 100644
> > > > --- a/tests/spec/arb_shader_image_load_store/execution/basic
> > > > -imageStore-from-uniform.shader_test
> > > > +++ b/tests/spec/arb_shader_image_load_store/execution/basic
> > > > -imageStore-from-uniform.shader_test
> > > > @@ -43,6 +43,7 @@ fb tex 2d 1
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 1.0 0.0 0.0 1.0
> > > >  
> > > > @@ -52,5 +53,6 @@ fb tex 2d 1
> > > >  draw rect -1 -1 2 2
> > > >  
> > > >  # Test the result of imageStore
> > > > +memory barrier GL_FRAMEBUFFER_BARRIER_BIT
> > > >  fb tex 2d 0
> > > >  probe all rgba 0.0 1.0 0.0 1.0
> > > > _______________________________________________
> > > > Piglit mailing list
> > > > Piglit at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit

More information about the Piglit mailing list