[Mesa-dev] [PATCH] r600/sb: fix rotated register in while loop (attempt 2)

[Gert Wollny]

Am Freitag, den 16.02.2018, 14:44 +1000 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> A bunch of CTS tests led me to write
> tests/shaders/ssa/fs-while-loop-rotate-value.shader_test
> which r600/sb always fell over on.
> 
> GCM seems to move some of the copys into other basic blocks,
> if we don't allow this to happen then it doesn't seem to schedule
> them badly.
> 
> Everything I've read on SSA/phi copies say they have to happen
> in parallel, so keeping them in the same basic block seems like
> a good way to keep some of that property.


This one fixes fs-while-loop-rotate-value for me (barts) as promised,
no longer fixes 

   spec/arb_enhanced_layouts/execution/component-layout/
        sso-vs-gs-fs-array-interleave

 but breaks  

   spec/glsl-1.50/execution/variable-indexing/
        vs-output-array-vec2-index-wr-before-gs


I don't know whether this is helpful, but when I apply a patch to TGSI
split arrays [1], this piglit also passes with your patch.
Specifically, since the loops are completely unrolled no indirect
access is happening and all arrays can be split. As a result, when
using plain master the shaders optimised by sb and when using your
patch on top of the array splitting are very similar (mostly
differently ordered literals and register indices), however, running 
master + this patch results in very different optimised shaders, e.g.
sb bundles many vfetch operations at the beginning.

Best, 
Gert 



[1] https://patchwork.freedesktop.org/series/37991/

> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/gallium/drivers/r600/sb/sb_shader.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/r600/sb/sb_shader.cpp
> b/src/gallium/drivers/r600/sb/sb_shader.cpp
> index 321e24ea25..8959b8391d 100644
> --- a/src/gallium/drivers/r600/sb/sb_shader.cpp
> +++ b/src/gallium/drivers/r600/sb/sb_shader.cpp
> @@ -121,7 +121,7 @@ alu_node* shader::create_copy_mov(value* dst,
> value* src, unsigned affcost) {
>  	alu_node *n = create_mov(dst, src);
>  
>  	dst->assign_source(src);
> -	n->flags |= NF_COPY_MOV | NF_DONT_HOIST;
> +	n->flags |= NF_COPY_MOV | NF_DONT_HOIST | NF_DONT_MOVE;
>  
>  	if (affcost && dst->is_sgpr() && src->is_sgpr())
>  		coal.add_edge(src, dst, affcost);