[Mesa-dev] [PATCH 1/3] i965/vec4/gs: restore the uniform values which was overwritten by failed vec4_gs_visitor execution

[Samuel Iglesias Gonsálvez]

Kind reminder that patches 1 and 3 are still unreviewed.

Sam

On Fri, 2017-05-05 at 12:38 +0200, Samuel Iglesias Gonsálvez wrote:
> We are going to add a packing feature to reduce the usage of the push
> constant buffer. One of the consequences is that 'nr_params' would be
> modified by vec4_visitor's run call, so we need to restore it if one
> of
> them failed before executing the fallback ones. Same thing happens to
> the
> uniforms values that would be reordered afterwards.
> 
> Fixes GL45-CTS.arrays_of_arrays_gl.InteractionFunctionCalls2 when
> the dvec4 alignment and packing patch is applied.
> 
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> Cc: "17.1" <mesa-stable at lists.freedesktop.org>
> ---
>  src/intel/compiler/brw_vec4_gs_visitor.cpp | 26
> ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp
> b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> index 4a8b5be30e..5fcd02831a 100644
> --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> @@ -868,10 +868,36 @@ brw_compile_gs(const struct brw_compiler
> *compiler, void *log_data,
>  
>           vec4_gs_visitor v(compiler, log_data, &c, prog_data,
> shader,
>                             mem_ctx, true /* no_spills */,
> shader_time_index);
> +
> +         /* Backup 'nr_params' and 'param' as they can be modified
> by the
> +          * the DUAL_OBJECT visitor. If it fails, we will run the
> fallback
> +          * (DUAL_INSTANCED or SINGLE mode) and we need to restore
> original
> +          * values.
> +          */
> +         const unsigned param_count = prog_data-
> >base.base.nr_params;
> +         gl_constant_value **param = ralloc_array(NULL,
> gl_constant_value*,
> +                                                  param_count);
> +         memcpy(param, prog_data->base.base.param,
> +                sizeof(gl_constant_value*) * param_count);
> +
>           if (v.run()) {
> +            /* Success! Backup is not needed */
> +            ralloc_free(param);
>              return brw_vec4_generate_assembly(compiler, log_data,
> mem_ctx,
>                                                shader, &prog_data-
> >base, v.cfg,
>                                                final_assembly_size);
> +         } else {
> +            /* These variables could be modified by the execution of
> the GS
> +             * visitor if it packed the uniforms in the push
> constant buffer.
> +             * As it failed, we need restore them so we can start
> again with
> +             * DUAL_INSTANCED or SINGLE mode.
> +             *
> +             * FIXME: Could more variables be modified by this
> execution?
> +             */
> +            memcpy(prog_data->base.base.param, param,
> +                   sizeof(gl_constant_value*) * param_count);
> +            prog_data->base.base.nr_params = param_count;
> +            ralloc_free(param);
>           }
>        }
>     }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170517/fd872c1e/attachment.sig>