[Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms

[Ian Romanick]

On 08/08/2014 01:56 PM, Neil Roberts wrote:
> The i965 driver uses a float pointer to point to the value of a uniform and
> also as the destination within the batch buffer. However the same locations
> can also be used to store values for integer uniforms. Previously the value
> was being copied into the batch buffer with a regular assignment. This breaks
> if the compiler does this by loading and saving through an x87 register
> because the fst instruction tries to fix up invalid float values. That can
> corrupt some specific integer values. This patch changes it to use a memcpy
> instead so that it won't use a floating-point register.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81150

There was a lot of chatter about this on IRC.  Glad it was resolved!

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

If this code also exists on the 10.2 branch, this should be tagged

Cc: mesa-stable at lists.freedesktop.org

> ---
>  src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index 905e123..cdbc083 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
>         * wouldn't be set for them.
>        */
>        for (i = 0; i < prog_data->nr_params; i++) {
> -         param[i] = *prog_data->param[i];
> +         /* A memcpy is used here instead of a regular assignment because
> +          * otherwise the value may end up being copied through a
> +          * floating-point register. This will break if the x87 registers are
> +          * used and we are storing an integer value here because the fst
> +          * instruction tries to fix up invalid values and that would corrupt
> +          * an integer value */
> +         memcpy(param + i, prog_data->param[i], sizeof param[i]);
>        }
>  
>        if (0) {
>