On 10/27/2016 06:02 AM, Nicolai Hähnle wrote:
> On 26.10.2016 02:59, Ian Romanick wrote:
>> From: Dave Airlie <airl...@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 <airl...@redhat.com>
>> Reviewed-by: Ian Romanick <ian.d.roman...@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.
>> + }
>> default:
>> assert(!"Should not get here.");
>> break;
>> @@ -453,12 +486,45 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
>> program, GLint location,
>> dst[didx].i = IROUNDD(tmp);
>> break;
>> }
>> + case GLSL_TYPE_UINT64: {
>> + uint64_t tmp;
>> + memcpy(&tmp, &src[sidx].u, sizeof(tmp));
>> + dst[didx].i = tmp;
>> + break;
>> + }
>> + case GLSL_TYPE_INT64: {
>> + int64_t tmp;
>> + memcpy(&tmp, &src[sidx].i, sizeof(tmp));
>> + dst[didx].i = tmp;
>> + break;
>> + }
>> default:
>> assert(!"Should not get here.");
>> break;
>> }
>> break;
>> -
>> + case GLSL_TYPE_INT64:
>> + case GLSL_TYPE_UINT64:
>> + switch (uni->type->base_type) {
>> + case GLSL_TYPE_UINT:
>> + *(int64_t *)&dst[didx].u = (int64_t) src[sidx].u;
>> + break;
>> + case GLSL_TYPE_INT:
>> + case GLSL_TYPE_SAMPLER:
>> + case GLSL_TYPE_IMAGE:
>> + *(int64_t *)&dst[didx].u = (int64_t) src[sidx].i;
>> + break;
>> + case GLSL_TYPE_BOOL:
>> + *(int64_t *)&dst[didx].u = src[sidx].i ? 1.0f : 0.0f;
>> + break;
>> + case GLSL_TYPE_FLOAT:
>> + *(int64_t *)&dst[didx].u = (int64_t) src[sidx].f;
>
> Similarly here, the .u is really quite meaningless.
>
> The rest looks good to me.
>
> Cheers,
> Nicolai
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
