[Mesa-dev] [PATCH 07/17] glsl/ir: Add builtin constant function support for doubles

[Ilia Mirkin]

On Thu, Feb 5, 2015 at 1:57 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Thu, Feb 5, 2015 at 3:05 AM, Topi Pohjolainen
> <topi.pohjolainen at intel.com> wrote:
>> From: Dave Airlie <airlied at gmail.com>
>>
>> (was: add double support)
>
> I was going to suggest removing this from the commit subjects, but
> just so you don't miss this one, remove it too. :)

Yeah, these are all gone.

>
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  src/glsl/ir_constant_expression.cpp | 234 +++++++++++++++++++++++++++++++-----
>>  1 file changed, 202 insertions(+), 32 deletions(-)
>>
>> diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp
>> index 1e8b3a3..4387a51 100644
>> --- a/src/glsl/ir_constant_expression.cpp
>> +++ b/src/glsl/ir_constant_expression.cpp
>> @@ -60,7 +60,7 @@ static double copysign(double x, double y)
>>  #endif
>>
>>  static float
>> -dot(ir_constant *op0, ir_constant *op1)
>> +dot_f(ir_constant *op0, ir_constant *op1)
>>  {
>>     assert(op0->type->is_float() && op1->type->is_float());
>>
>> @@ -71,6 +71,18 @@ dot(ir_constant *op0, ir_constant *op1)
>>     return result;
>>  }
>>
>> +static double
>> +dot_d(ir_constant *op0, ir_constant *op1)
>> +{
>> +   assert(op0->type->is_double() && op1->type->is_double());
>> +
>> +   double result = 0;
>> +   for (unsigned c = 0; c < op0->type->components(); c++)
>> +      result += op0->value.d[c] * op1->value.d[c];
>> +
>> +   return result;
>> +}
>> +
>>  /* This method is the only one supported by gcc.  Unions in particular
>>   * are iffy, and read-through-converted-pointer is killed by strict
>>   * aliasing.  OTOH, the compiler sees through the memcpy, so the
>> @@ -667,32 +679,75 @@ ir_expression::constant_expression_value(struct hash_table *variable_context)
>>             data.b[0] = true;
>>        }
>>        break;
>> -
>> -   case ir_unop_trunc:
>> +   case ir_unop_d2f:
>> +      assert(op[0]->type->base_type == GLSL_TYPE_DOUBLE);
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +        data.f[c] = op[0]->value.d[c];
>> +      }
>> +      break;
>> +   case ir_unop_f2d:
>>        assert(op[0]->type->base_type == GLSL_TYPE_FLOAT);
>>        for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> -        data.f[c] = truncf(op[0]->value.f[c]);
>> +        data.d[c] = op[0]->value.f[c];
>> +      }
>> +      break;
>> +   case ir_unop_d2i:
>> +      assert(op[0]->type->base_type == GLSL_TYPE_DOUBLE);
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +        data.i[c] = op[0]->value.d[c];
>> +      }
>> +      break;
>> +   case ir_unop_i2d:
>> +      assert(op[0]->type->base_type == GLSL_TYPE_INT);
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +        data.d[c] = op[0]->value.i[c];
>> +      }
>> +      break;
>> +   case ir_unop_d2u:
>> +      assert(op[0]->type->base_type == GLSL_TYPE_DOUBLE);
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +        data.u[c] = op[0]->value.d[c];
>> +      }
>> +      break;
>> +   case ir_unop_u2d:
>
> Missing b2d/d2b.

They don't exist...

>
>> +      assert(op[0]->type->base_type == GLSL_TYPE_UINT);
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +        data.d[c] = op[0]->value.u[c];
>> +      }
>> +      break;
>> +   case ir_unop_trunc:
>> +      for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> +         if (op[0]->type->base_type == GLSL_TYPE_DOUBLE)
>> +            data.d[c] = trunc(op[0]->value.d[c]);
>> +         else
>> +            data.f[c] = truncf(op[0]->value.f[c]);
>>        }
>>        break;
>>
>>     case ir_unop_round_even:
>> -      assert(op[0]->type->base_type == GLSL_TYPE_FLOAT);
>>        for (unsigned c = 0; c < op[0]->type->components(); c++) {
>> -        data.f[c] = _mesa_round_to_even(op[0]->value.f[c]);
>> +         if (op[0]->type->base_type == GLSL_TYPE_DOUBLE)
>> +            data.d[c] = _mesa_round_to_even(op[0]->value.d[c]);
>
> _mesa_round_to_even takes a float. This can't work.
>
> And, looking at the implementation and the comment above it... can't
> we do better and actually implement a real round-to-even function?

_mesa_round_to_even returns an int, so it can't work for floats
either. I don't think fixing this needs to be part of the series...
it'll just fail horribly for too-big things.

>
>> +         else
>> +            data.f[c] = _mesa_round_to_even(op[0]->value.f[c]);
>>        }
>>        break;
>>
>> @@ -1474,6 +1600,17 @@ ir_expression::constant_expression_value(struct hash_table *variable_context)
>>           data.f[c] = CLAMP(op[0]->value.f[c], 0.0f, 1.0f);
>>        }
>>        break;
>> +   case ir_unop_pack_double_2x32: {
>> +      uint64_t temp;
>> +      temp = (uint64_t)op[0]->value.u[0] | ((uint64_t)op[0]->value.u[1] << 32);
>> +      data.d[0] = *(double *)&temp;
>
> I'm not sure if this is safe for big endian. Would a memcpy be endian
> safe (and also avoid breaking strict aliasing rules)?

>From https://www.opengl.org/sdk/docs/man/html/packDouble2x32.xhtml :

"""
The first vector component (v[0]) specifies the 32 least significant
bits of the result; the second component (v[1]) specifies the 32 most
significant bits.
"""

This stuff gives me a headache. Does it mean that we need to byteswap
the resulting temp on BE machines? Either way, I'm pretty sure that a
plain memcpy wouldn't help here.

>
>> +
>> +      break;
>> +   }
>> +   case ir_unop_unpack_double_2x32:
>> +      data.u[0] = *(uint32_t *)&op[0]->value.d[0];
>> +      data.u[1] = *((uint32_t *)&op[0]->value.d[0] + 1);
>
> Same comment about memcpy.

Same comment about headache ;) Happy to just stick a XXX in there and
move on. Thoughts?

>
>> +      break;
>>
>>     case ir_triop_bitfield_extract: {
>>        int offset = op[1]->value.i[0];