[Mesa-dev] [PATCH 0/9] glsl: cleanup and fix handling of unnamed struct types

[Nicolai Hähnle]

Final ping. If I don't hear anything, I'll push this with the changes 
requested by Timothy at the end of the week.

Cheers,
Nicolai

On 22.05.2017 13:31, Nicolai Hähnle wrote:
> Hey Ian,
> 
> Do you want more time to look into this? I double-checked that glslang 
> accepts this code pattern.
> 
> Cheers,
> Nicolai
> 
> On 16.05.2017 14:29, Nicolai Hähnle wrote:
>> On 16.05.2017 02:56, Ian Romanick wrote:
>>> On 05/15/2017 02:27 AM, Nicolai Hähnle wrote:
>>>> Hi all,
>>>>
>>>> This series aims to simplify how we handle unnamed struct types and
>>>> fix some
>>>> bugs on the way. Some of the patches should be uncontroversial, and
>>>> all of
>>>> them align with my understanding of the GLSL spec, but the spec isn't
>>>> exactly explicit, so...
>>>>
>>>> At a high level, the series changes two things:
>>>>
>>>> 1) Unnamed structs can never match named structs during linking.
>>>>
>>>> 2) Unnamed structs are considered the same type if they have the same
>>>> content. They will literally use the same glsl_type instance.
>>>>
>>>> The second point causes a deviation from the behavior of C/C++, in
>>>> that the
>>>> following code snippet now compiles (and does the reasonable thing):
>>>>
>>>>   struct { float a; } s1;
>>>>   struct { float a; } s2;
>>>>
>>>>   s2 = s1;
>>>
>>> I'll have to do some archaeology, but I believe this is what we used to
>>> do.  I believe that we changed it because we were required to do so.
>>>
>>> Does this fix some app, or is it just to simplify the code?
>>
>> It's just to simplify the code (or rather: to fix what I'd consider a
>> bug without making things *much* more complicated). Erik wrote in a
>> separate reply that he tested that code snippet on other
>> implementations, and they accept it. I didn't notice regressions in my
>> own testing yet.
>>
>> Cheers,
>> Nicolai
>>
>>
>>>
>>>> The justification is basically that the GLSL spec says (when it 
>>>> comes to
>>>> linking, in Section 4.2) that
>>>>
>>>>    "Structures must have the same name, sequence of type names, and
>>>>     type definitions, and member names to be considered the same type."
>>>>
>>>> And since people generally seem to read that as mandating that
>>>> globals of
>>>> unnamed struct types should match if they're structurally the same,
>>>> and the
>>>> GLSL spec otherwise says nothing (as far as I can tell) about when
>>>> struct
>>>> types are equal, the cleanest thing is to just say that _in general_
>>>> (not
>>>> just for linking) struct types are the same if and only if they have 
>>>> the
>>>> same name and the same fields; where "same name" can mean that they 
>>>> both
>>>> have no name.
>>>>
>>>> Please contemplate and review! :)
>>>> Thanks,
>>>> Nicolai
>>>> -- 
>>>>  src/compiler/glsl/ast.h                   |  1 +
>>>>  src/compiler/glsl/ast_to_hir.cpp          | 17 ++---
>>>>  src/compiler/glsl/glsl_parser_extras.cpp  | 17 +++--
>>>>  src/compiler/glsl/link_varyings.cpp       | 41 +++---------
>>>>  src/compiler/glsl/linker.cpp              | 47 +++++---------
>>>>  src/compiler/glsl/lower_ubo_reference.cpp |  2 +-
>>>>  src/compiler/glsl_types.cpp               | 69 +++++++++------------
>>>>  src/compiler/glsl_types.h                 | 11 ++--
>>>>  8 files changed, 80 insertions(+), 125 deletions(-)
>>>>
>>>> _______________________________________________
>>>> 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.