[Mesa-dev] [PATCH 0/4] i965: skip control-flow aware liveness analysis if we only have 1 block

[Iago Toral]

On Tue, 2015-10-13 at 09:44 -0700, Jordan Justen wrote:
> On 2015-10-13 05:17:37, Francisco Jerez wrote:
> > Iago Toral Quiroga <itoral at igalia.com> writes:
> > 
> > > This fixes the following test:
> > >
> > > [require]
> > > GL >= 3.3
> > > GLSL >= 3.30
> > > GL_ARB_shader_storage_buffer_object
> > >
> > > [fragment shader]
> > > #version 330
> > > #extension GL_ARB_shader_storage_buffer_object: require
> > >
> > > buffer SSBO {
> > >     mat4 sm4;
> > > };
> > >
> > >
> > > mat4 um4;
> > >
> > > void main() {
> > >     sm4 *= um4;
> > 
> > This is using the value of "um4", which is never assigned, so liveness
> > analysis will correctly extend its live interval up to the start of the
> > block.
> 
> This test was derived by simplifying a CTS test case.
> 
> Anyway, I'm not sure what happened on the way to the commit message,
> but um4 should be a uniform.
> 
> http://sprunge.us/cEUe

Oh yes, that was me playing around with the example. The patches also
fix the uniform version. Jordan, can you verify if this fixes the CTS
test case?

In any case, since Curro is working on a more general fix for this
stuff, I guess you'd rather wait for his patches...

Iago

> -Jordan
> 
> > The other problem here seems to be that the liveness analysis pass
> > doesn't consider scalar writes (like the ones emitted by the
> > combine_constants optimization pass and by emit_uniformize()) to fully
> > define the contents of a register, so it will incorrectly extend up the
> > live interval of registers defined using scalar writes even if all
> > components ever used in the shader have been defined individually.
> > Incidentally the use-def-chains-based implementation of liveness
> > analysis I'm working on will fix this issue easily.
> > 
> > >     sm4[0][0] = 0.0;
> > > }
> > >
> > > [test]
> > > link success
> > >
> > > where our liveness analysis works really bad because the control-flow part
> > > will expand register liveness to the end of only block we have (so every
> > > register will be marked alive until the end of the program). This artificially
> > > increases register pressure to a point in which we run out of registers.
> > > Of course, this is only a simple optimization for a trivial case, the
> > > underlying problem still exists and would manifest when we have more than
> > > one block, but it helps simple shaders such as the one in the example above
> > > without any effort. I guess the real fix would involve re-thinking parts of the
> > > liveness analysis algorithm...
> > >
> > > Jordan, there is another thing that we can improve for this shader that is
> > > specific to SSBOs: we emit the same ssbo load multiple times because we are
> > > playing it safe just in case there are writes in between. I think we can try to
> > > do better and not re-issue the same load if we don't have ssbo stores to
> > > the same address in between. I'll try to come up with a patch for this
> > > (hopefully we can do this inside lower_ubo_reference).
> > >
> > > The actual fix goes into patches 2 (FS) and 4 (VS). Patches 1,3 are just
> > > indentation fixes in the code around these.
> > >
> > > Iago Toral Quiroga (4):
> > >   i965/fs: Fix indentation in fs_live_variables::compute_start_end
> > >   i965/fs: skip control-flow aware liveness analysis if we only have 1
> > >     block
> > >   i965/vec4: fix indentation in vec4_visitor::calculate_live_intervals
> > >   i965/vec4: skip control-flow aware liveness analysis if we only have 1
> > >     block
> > >
> > >  .../drivers/dri/i965/brw_fs_live_variables.cpp     | 24 ++++++++++++++--------
> > >  .../drivers/dri/i965/brw_vec4_live_variables.cpp   | 23 +++++++++++++--------
> > >  2 files changed, 30 insertions(+), 17 deletions(-)
> > >
> > > -- 
> > > 1.9.1
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>