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

[Francisco Jerez]

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.

>     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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151013/d8591473/attachment-0001.sig>