[Mesa-dev] [PATCH v2 30/31] glsl: disable tree grafting optimization for bindless images

[Nicolai Hähnle]

On 28.04.2017 03:15, Timothy Arceri wrote:
> On 26/04/17 18:27, Samuel Pitoiset wrote:
>> On 04/26/2017 10:05 AM, Nicolai Hähnle wrote:
>>> On 24.04.2017 12:36, Samuel Pitoiset wrote:
>>>> Because the variable declaration holds more information than
>>>> the dereference. Note that an image is considered bindless either
>>>> if it has been declared in the default uniform block with the
>>>> bindless_sampler layout qualifier, or when its storage is not
>>>> uniform because this is not allowed without ARB_bindless_texture.
>>>
>>> It seems unfortunate that we have to do this. Can you explain what
>>> goes wrong without this change?
>>
>> We lost the variable and all information contained in ir_variable for
>> images.
>
> Can you give an example of a shader where this becomes an issue? And why?

Samuel provided me with one, but the basic problem is actually quite 
simple. Consider this (using GLSL rather than the IR, but I hope you get 
the drift):

   coherent image2D img = (some expression);
   out = imageLoad(img, ...);

Tree grafting will convert that to

   out = imageLoad((some expression), ...);

But now the coherent qualifier is gone.

In a way, GLSL is not very well specified here: the qualifiers should 
really be part of the type, at least in the same way that C/C++ have the 
cv-qualifiers, but they aren't.

There is another problem, which is the "mere" implementation problem 
that st_glsl_to_tgsi is only set up to handle sampler/image parameters 
to intrinsics that are direct dereferences. visit_image_intrinsics has a 
cast from ir_rvalue to ir_dereference, which is simply incorrect when 
that parameter is an expression.

The easy answer is to just not do tree grafting for samplers and images.

The cleaner answer is to disable tree grafting only when any of the 
data.image_* qualifiers are set on the variable to be grafted, and to 
fix st_glsl_to_tgsi so that it also handles expressions as sampler/image 
parameters to intrinsics.

Cheers,
Nicolai



>
>>
>>>
>>> Thanks,
>>> Nicolai
>>>
>>>
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>  src/compiler/glsl/opt_tree_grafting.cpp | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/src/compiler/glsl/opt_tree_grafting.cpp
>>>> b/src/compiler/glsl/opt_tree_grafting.cpp
>>>> index 28b6e1856e..d4a1ec5675 100644
>>>> --- a/src/compiler/glsl/opt_tree_grafting.cpp
>>>> +++ b/src/compiler/glsl/opt_tree_grafting.cpp
>>>> @@ -371,6 +371,15 @@ tree_grafting_basic_block(ir_instruction
>>>> *bb_first,
>>>>        if (lhs_var->data.precise)
>>>>           continue;
>>>>
>>>> +      if (lhs_var->type->is_image() &&
>>>> +          (lhs_var->data.bindless || lhs_var->data.mode !=
>>>> ir_var_uniform)) {
>>>> +         /* Disable tree grafting optimization for bindless image
>>>> types because
>>>> +          * the variable declaration holds more information than the
>>>> +          * dereference.
>>>> +          */
>>>> +         continue;
>>>> +      }
>>>> +
>>>>        ir_variable_refcount_entry *entry =
>>>> info->refs->get_variable_entry(lhs_var);
>>>>
>>>>        if (!entry->declaration ||
>>>>
>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.