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

[Nicolai Hähnle]

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.