[Mesa-dev] [PATCH 2/8] glsl: Add wrapper function that calls ir_dereference::constant_referenced

[Ian Romanick]

On 03/12/2014 04:20 PM, Connor Abbott wrote:
> On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/glsl/ir_constant_expression.cpp | 52 +++++++++++++++++++++++++------------
>>  1 file changed, 36 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp
>> index a31e579..4149a0e 100644
>> --- a/src/glsl/ir_constant_expression.cpp
>> +++ b/src/glsl/ir_constant_expression.cpp
>> @@ -395,6 +395,37 @@ unpack_half_1x16(uint16_t u)
>>   * The offset is used when the reference is to a specific column of a matrix.
>>   */
>>  /*@{*/
>> +static bool
>> +constant_referenced(const ir_dereference *deref,
>> +                    struct hash_table *variable_context,
>> +                    ir_constant *&store, int &offset)
>> +{
>> +   switch (deref->ir_type) {
>> +   case ir_type_dereference_array:
>> +      ((ir_dereference_array *) deref)->constant_referenced(variable_context,
>> +                                                            store, offset);
>> +      break;
>> +
>> +   case ir_type_dereference_record:
>> +      ((ir_dereference_record *) deref)->constant_referenced(variable_context,
>> +                                                             store, offset);
>> +      break;
>> +
>> +   case ir_type_dereference_variable:
>> +      ((ir_dereference_variable *) deref)->constant_referenced(variable_context,
>> +                                                               store, offset);
>> +      break;
>> +
>> +   default:
>> +      assert(!"Should not get here.");
>> +      store = NULL;
>> +      offset = 0;
>> +      break;
>> +   }
>> +
>> +   return store != NULL;
>> +}
>> +
>>  void
>>  ir_dereference_variable::constant_referenced(struct hash_table *variable_context,
>>                                              ir_constant *&store, int &offset) const
>> @@ -433,13 +464,8 @@ ir_dereference_array::constant_referenced(struct hash_table *variable_context,
>>        return;
>>     }
>>
>> -   deref->constant_referenced(variable_context, substore, suboffset);
>> -
>> -   if (!substore) {
>> -      store = 0;
>> -      offset = 0;
>> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>>        return;
>> -   }
> 
> Question: why do you have the :: here...

Without it GCC was trying to resolve constant_referenced to
ir_dereference::constant_referenced.  Since there isn't one that
matches this signatrue, it gave an error.

../../src/glsl/ir_constant_expression.cpp: In member function 'virtual void ir_dereference_array::constant_referenced(hash_table*, ir_constant*&, int&) const':
../../src/glsl/ir_constant_expression.cpp:467:73: error: no matching function for call to 'ir_dereference_array::constant_referenced(const ir_dereference*&, hash_table*&, ir_constant*&, int&) const'
../../src/glsl/ir_constant_expression.cpp:467:73: note: candidate is:
../../src/glsl/ir_constant_expression.cpp:443:1: note: virtual void ir_dereference_array::constant_referenced(hash_table*, ir_constant*&, int&) const
../../src/glsl/ir_constant_expression.cpp:443:1: note:   candidate expects 3 arguments, 4 provided

It was a little annoying, but having a typo lead to a completely
different function being called would be another sort of annoying
bug... so I didn't curse it too much. :)

>>
>>     const glsl_type *vt = array->type;
>>     if (vt->is_array()) {
>> @@ -475,13 +501,8 @@ ir_dereference_record::constant_referenced(struct hash_table *variable_context,
>>        return;
>>     }
>>
>> -   deref->constant_referenced(variable_context, substore, suboffset);
>> -
>> -   if (!substore) {
>> -      store = 0;
>> -      offset = 0;
>> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>>        return;
>> -   }
> 
> and here...
> 
>>
>>     store = substore->get_record_field(field);
>>     offset = 0;
>> @@ -1814,9 +1835,8 @@ bool ir_function_signature::constant_expression_evaluate_expression_list(const s
>>
>>          ir_constant *store = NULL;
>>          int offset = 0;
>> -        asg->lhs->constant_referenced(variable_context, store, offset);
>>
>> -        if (!store)
>> +        if (!constant_referenced(asg->lhs, variable_context, store, offset))
>>             return false;
> 
> but not here...

This location isn't in the ir_deference class hierarchy, so it didn't
need the scope resolution.  I suspect this point may have failed to
compile with it.

>>
>>          ir_constant *value = asg->rhs->constant_expression_value(variable_context);
>> @@ -1847,9 +1867,9 @@ bool ir_function_signature::constant_expression_evaluate_expression_list(const s
>>
>>          ir_constant *store = NULL;
>>          int offset = 0;
>> -        call->return_deref->constant_referenced(variable_context, store, offset);
>>
>> -        if (!store)
>> +        if (!constant_referenced(call->return_deref, variable_context,
>> +                                  store, offset))
> 
> or here? Compiler issues?
> 
>>             return false;
>>
>>          ir_constant *value = call->constant_expression_value(variable_context);
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev