[Mesa-dev] [PATCH 5/5] glsl: Add frexp signatures and implementation.

[Matt Turner]

On Wed, Sep 11, 2013 at 10:03 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 9 September 2013 15:14, Matt Turner <mattst88 at gmail.com> wrote:
>>
>> I initially implemented frexp() as an IR opcode with a lowering pass,
>> but since it returns a value and has an out-parameter, it would break
>> assumptions our optimization passes make about ir_expressions being pure
>> (i.e., having no side effects).
>>
>> For example, if opt_tree_grafting encounters this code:
>>
>> uniform float u;
>> void main()
>> {
>>   int exp;
>>   float f = frexp(u, out exp);
>>   float g = float(exp)/256.0;
>>   float h = float(exp) + 1.0;
>>   gl_FragColor = vec4(f, g, h, g + h);
>> }
>>
>> it may try to optimize it to this:
>>
>> uniform float u;
>> void main()
>> {
>>   int exp;
>>   float g = float(exp)/256.0;
>>   float h = float(exp) + 1.0;
>>   gl_FragColor = vec4(frexp(u, out exp), g, h, g + h);
>> }
>>
>> Some hardware has an instruction which performs frexp(), but we would
>> need some other compiler infrastructure to be able to generate it, such
>> as an intrinsics system that would allow backends to emit specific code
>> for particular bits of IR.
>> ---
>>  src/glsl/builtin_functions.cpp | 54
>> ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/src/glsl/builtin_functions.cpp
>> b/src/glsl/builtin_functions.cpp
>> index dbd35f2..e9d7b74 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -512,6 +512,7 @@ private:
>>     B1(findMSB)
>>     B1(fma)
>>     B2(ldexp)
>> +   B2(frexp)
>>  #undef B0
>>  #undef B1
>>  #undef B2
>> @@ -1828,6 +1829,13 @@ builtin_builder::create_builtins()
>>                  _ldexp(glsl_type::vec3_type,  glsl_type::ivec3_type),
>>                  _ldexp(glsl_type::vec4_type,  glsl_type::ivec4_type),
>>                  NULL);
>> +
>> +   add_function("frexp",
>> +                _frexp(glsl_type::float_type, glsl_type::int_type),
>> +                _frexp(glsl_type::vec2_type,  glsl_type::ivec2_type),
>> +                _frexp(glsl_type::vec3_type,  glsl_type::ivec3_type),
>> +                _frexp(glsl_type::vec4_type,  glsl_type::ivec4_type),
>> +                NULL);
>>  #undef F
>>  #undef FI
>>  #undef FIU
>> @@ -3524,6 +3532,52 @@ builtin_builder::_ldexp(const glsl_type *x_type,
>> const glsl_type *exp_type)
>>  {
>>     return binop(ir_binop_ldexp, gpu_shader5, x_type, x_type, exp_type);
>>  }
>> +
>> +ir_function_signature *
>> +builtin_builder::_frexp(const glsl_type *x_type, const glsl_type
>> *exp_type)
>> +{
>> +   ir_variable *x = in_var(x_type, "x");
>> +   ir_variable *exponent = out_var(exp_type, "exp");
>> +   MAKE_SIG(x_type, gpu_shader5, 2, x, exponent);
>> +
>> +   const unsigned vec_elem = x_type->vector_elements;
>> +   const glsl_type *bvec = glsl_type::get_instance(GLSL_TYPE_BOOL,
>> vec_elem, 1);
>> +   const glsl_type *uvec = glsl_type::get_instance(GLSL_TYPE_UINT,
>> vec_elem, 1);
>> +
>> +   /* Single-precision floating-point values are stored as
>> +    *   1 sign bit;
>> +    *   8 exponent bits;
>> +    *   23 mantissa bits.
>> +    *
>> +    * An exponent shift of 23 will shift the mantissa out, leaving only
>> the
>> +    * exponent and sign bit (which itself may be zero, if the absolute
>> value
>> +    * was taken before the bitcast and shift.
>> +    */
>> +   ir_constant *exponent_shift = imm(23);
>> +   ir_constant *exponent_bias = imm(-126, vec_elem);
>> +
>> +   ir_constant *sign_mantissa_mask = imm(0x807fffffu, vec_elem);
>> +   ir_constant *exponent_mask = imm(0x3f000000u, vec_elem);
>
>
> Actually the exponent mask would be 0x7f800000u.  This is the exponent
> *value* corresponding to a float in the range [0.5, 1.0).  Fortunately
> that's what we use it for :).  I'd propose renaming it to something like
> "exponent_value", and maybe adding an explanatory comment.

Indeed you are correct. Thanks for catching this, I'll rename it and
add a comment.

Thanks for the review, and sorry I'm not around this week to do the same.
Matt