[Mesa-dev] [PATCH 5/5] Change ir_function_signature::constant_expression_value to run through the function.

[Kenneth Graunke]

On 04/30/2012 11:45 AM, Eric Anholt wrote:
> On Fri, 27 Apr 2012 10:28:04 +0200, Olivier Galibert<galibert at pobox.com>  wrote:
>> That removes code duplication with
>> ir_expression::constant_expression_value and builtins/ir/*.
>
> I'm concerned that this will turn things that shouldn't be constant
> expressions in GLSL (some user-defined function just using its
> (constant) arguments and constants) and treating them as constant
> expressions.  Do we have tests for that?

I'm pretty sure we do, and if you notice the very top of the function 
(unchanged by Olivier's patch):

    if (!this->is_builtin)
       return NULL;

So we'll only try to do this for built-in functions.

>>   src/glsl/ir_constant_expression.cpp |  450 +++++++----------------------------
>>   1 file changed, 80 insertions(+), 370 deletions(-)
>>
>> diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp
>> index ee1cc1a..a7aafaa 100644
>> --- a/src/glsl/ir_constant_expression.cpp
>> +++ b/src/glsl/ir_constant_expression.cpp
>> @@ -1050,396 +1050,106 @@ ir_function_signature::constant_expression_value(exec_list *actual_parameters)
>>      if (!this->is_builtin)
>>         return NULL;
>>
>> -   unsigned num_parameters = 0;
>> +   /*
>> +    * Of the builtin functions, only the texture lookups and the noise
>> +    * ones must not be used in constant expressions.  They all include
>> +    * specific opcodes so they don't need to be special-cased at this
>> +    * point.
>> +    */
>> +
>> +   /* Initialize the table of dereferencable names with the function
>> +    * parameters.  Verify their const-ness on the way.
>> +    *
>> +    * We expect the correctness of the number of parameters to have
>> +    * been checked earlier.
>> +    */
>> +   hash_table *deref_hash = hash_table_ctor(8, hash_table_string_hash,
>> +					    hash_table_string_compare);
>
> Our variables don't generally have unique names.  It looks like from
> your usage, that you could use pointer_hash/compare and get cheaper
> searches plus unique references to variables.

Good catch.  While in the hand-coded built-in IR we typically use unique 
names, that isn't true for compiler-generated IR, which you use for 
inverse() now.  So we need to hash based on the ir_variable pointer.