[Mesa-dev] [PATCH 12/59] mesa: Add support for 64-bit integer uniforms. (v2)

[Dave Airlie]

On 11 November 2016 at 08:57, Ian Romanick <idr at freedesktop.org> wrote:
> On 10/27/2016 06:02 AM, Nicolai Hähnle wrote:
>> On 26.10.2016 02:59, Ian Romanick wrote:
>>> From: Dave Airlie <airlied at redhat.com>
>>>
>>> This hooks up the API to the internals for 64-bit integer uniforms.
>>>
>>> v2: update to use non-strict aliased alternatives
>>>
>>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>>> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>>> ---
>>>  src/mesa/main/uniform_query.cpp |  82 ++++++++++++++++++-
>>>  src/mesa/main/uniforms.c        | 170
>>> +++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 247 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/mesa/main/uniform_query.cpp
>>> b/src/mesa/main/uniform_query.cpp
>>> index db700df..8ecaef4 100644
>>> --- a/src/mesa/main/uniform_query.cpp
>>> +++ b/src/mesa/main/uniform_query.cpp
>> [snip]
>>> @@ -416,6 +433,22 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
>>> program, GLint location,
>>>                    memcpy(&dst[didx].f, &tmp, sizeof(tmp));
>>>            break;
>>>                 }
>>> +               case GLSL_TYPE_UINT64: {
>>> +                  uint64_t tmpu;
>>> +                  double tmp;
>>> +                  memcpy(&tmpu, &src[sidx].u, sizeof(tmpu));
>>> +                  tmp = tmpu;
>>> +                  memcpy(&dst[didx].f, &tmp, sizeof(tmp));
>>> +                  break;
>>> +               }
>>> +               case GLSL_TYPE_INT64: {
>>> +                  int64_t tmpi;
>>> +                  double tmp;
>>> +                  memcpy(&tmpi, &src[sidx].i, sizeof(tmpi));
>>> +                  tmp = tmpi;
>>> +                  memcpy(&dst[didx].f, &tmp, sizeof(tmp));
>>> +                  break;
>>
>> This pattern really looks quite nasty. Can we at least not pretend that
>> the .f is meaningful and use
>>
>>   memcpy(&dst[dix], ...);
>>
>> or maybe even *(double *)&dst[didx] = tmpi;?
>
> I think we need to do the memcpy (and should below too) to avoid
> strict-aliasing problems.  I don't recall all the details.  Perhaps Matt
> can offer some guidance.

Yes this needs to be a memcpy to avoid strict aliasing, we could .u
instead of .f
but it doesn't really matter.

Dave.