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

[Kenneth Graunke]

On Friday, August 08, 2014 02:56:53 PM Eric Anholt wrote:
> Roland Scheidegger <sroland at vmware.com> writes:
> 
> > Am 08.08.2014 22:56, schrieb Neil Roberts:
> >> 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://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D81150&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=5ZyizVpb8bGGN4qANo378HB6GFoFoBABoDXdPAyIszE%3D%0A&s=107a04b60233e66abfb9e83a62b90d128d60d6c61eb41b1a08ec7ab537283903
> >> ---
> >>  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]);
> >>        }
> 
> Or just change the pointer type to uint32_t, right?

I think I'd prefer that.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140808/83e1180c/attachment.sig>