[Mesa-dev] [PATCH] glsl: Avoid calling get_array_element for scalar constants

[Tapani Pälli]

On 08/14/2018 12:15 PM, Danylo Piliaiev wrote:
> Hi,
> 
> 
> On 08/14/2018 10:53 AM, Tapani Pälli wrote:
>> Hi;
>>
>> On 08/13/2018 06:57 PM, Danylo Piliaiev wrote:
>>> Accessing scalar constant as an array in function call or
>>> initializer list triggered assert in get_array_element.
>>> Examples:
>>>     func(0[0]);
>>>     vec2 t = { 0[0], 0 };
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
>>>
>>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
>>> ---
>>>   src/compiler/glsl/ir_constant_expression.cpp | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/compiler/glsl/ir_constant_expression.cpp 
>>> b/src/compiler/glsl/ir_constant_expression.cpp
>>> index 4a0aff72c6..c9788c7053 100644
>>> --- a/src/compiler/glsl/ir_constant_expression.cpp
>>> +++ b/src/compiler/glsl/ir_constant_expression.cpp
>>> @@ -826,7 +826,7 @@ 
>>> ir_dereference_array::constant_expression_value(void *mem_ctx,
>>>            const unsigned component = idx->value.u[0];
>>>              return new(mem_ctx) ir_constant(array, component);
>>> -      } else {
>>> +      } else if (array->type->is_array()) {
>>
>> IMO this is correct but we could alternatively bail out already 
>> earlier before the topmost if-statement or just add this condition as 
>> part of the topmost if-statement?
>>
> We don't have "accessible_as_array" kind of check (correct me if I'm 
> wrong), vector and matrix are not is_array so to bail out earlier there 
> should be a check:
> is_matrix || is_vector || is_array
> or add new method to glsl_type. But from my perspective this will only 
> invert conditions order in constant_expression_value.

Right, yeah I was trying to envision if we could simply break out 
earlier but it seems it might actually end up to be more complex ... I'm 
good with this one;

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

// Tapani