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

[Nicolai Hähnle]

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.