[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 15:17 +0300, 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.
> 
> 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.

Great, thanks Curro! Once you make your change public I can verify that
they fix this too.

BTW, even if your changes fix this I wonder if my patch is still valid:
control-flow analysis should not add anything relevant to liveness
analysis if we only have one block, right?

Iago

> >     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