[Mesa-dev] [PATCH 08/15] glsl: Switch ast_type_qualifier to a 128-bit bitset.

[Roland Scheidegger]

This seems to have broken compilation with some gcc versions (with scons
build):

In file included from src/compiler/glsl/ast_array_index.cpp:24:0:
src/compiler/glsl/ast.h:648:16: error: member
‘ast_type_qualifier::bitset_t ast_type_qualifier::flags::i’ with
constructor not allowed in union
       bitset_t i;
                ^
src/compiler/glsl/ast.h:648:16: note: unrestricted unions only available
with -std=c++11 or -std=gnu++11
scons: *** [build/linux-x86_64-checked/compiler/glsl/ast_array_index.os]
Error 1
src/gallium/tests/unit/u_format_test.c: In function ‘main’:
src/gallium/tests/unit/u_format_test.c:649:44: warning: array subscript
is above array bounds [-Warray-bounds]
          unpacked[i][j] = test->unpacked[i][j][1];
                                            ^
In file included from src/compiler/glsl/ast_expr.cpp:24:0:
src/compiler/glsl/ast.h:648:16: error: member
‘ast_type_qualifier::bitset_t ast_type_qualifier::flags::i’ with
constructor not allowed in union
       bitset_t i;
                ^
src/compiler/glsl/ast.h:648:16: note: unrestricted unions only available
with -std=c++11 or -std=gnu++11

Roland


Am 14.02.2018 um 22:18 schrieb Francisco Jerez:
> This should end the drought of bits in the ast_type_qualifier object.
> The bitset_t type works pretty much as a drop-in replacement for the
> current uint64_t bitset.
> 
> The only catch is that the bitset_t type as defined in the previous
> commit doesn't have a trivial constructor (because it has a
> user-defined constructor), so it cannot be used as union member
> without providing a user-defined constructor for the union (which
> causes it in turn to be non-trivially constructible).  This annoyance
> could be easily addressed in C++11 by declaring the default
> constructor of bitset_t to be the implicitly defined one -- IMO one
> more reason to drop support for GCC 4.2-4.3.
> 
> The other minor change was required because glsl_parser_extras.cpp was
> hard-coding the type of bitset temporaries as uint64_t, which (unlike
> would have been the case if the uint64_t had been replaced with
> e.g. an __int128) would otherwise have caused a build failure, because
> the boolean conversion operator of bitset_t is marked explicit (if
> C++11 is available), so the bitset won't be silently truncated down to
> 1 bit in order to use it to initialize the uint64_t temporaries
> (yikes).
> ---
>  src/compiler/glsl/ast.h                  | 8 ++++++--
>  src/compiler/glsl/glsl_parser.yy         | 1 +
>  src/compiler/glsl/glsl_parser_extras.cpp | 4 ++--
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index eee22482812..b0db14e20b6 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -28,6 +28,7 @@
>  #include "list.h"
>  #include "glsl_parser_extras.h"
>  #include "compiler/glsl_types.h"
> +#include "util/bitset.h"
>  
>  struct _mesa_glsl_parse_state;
>  
> @@ -473,8 +474,11 @@ enum {
>  
>  struct ast_type_qualifier {
>     DECLARE_RALLOC_CXX_OPERATORS(ast_type_qualifier);
> +   DECLARE_BITSET_T(bitset_t, 128);
> +
> +   union flags {
> +      flags() {}
>  
> -   union {
>        struct {
>  	 unsigned invariant:1;
>           unsigned precise:1;
> @@ -636,7 +640,7 @@ struct ast_type_qualifier {
>        q;
>  
>        /** \brief Set of flags, accessed as a bitmask. */
> -      uint64_t i;
> +      bitset_t i;
>     } flags;
>  
>     /** Precision of the type (highp/medium/lowp). */
> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
> index 19147c7a3ec..4faf9602a08 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -96,6 +96,7 @@ static bool match_layout_qualifier(const char *s1, const char *s2,
>  %parse-param {struct _mesa_glsl_parse_state *state}
>  
>  %union {
> +   YYSTYPE() {}
>     int n;
>     int64_t n64;
>     float real;
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index d99916d8ada..57589843776 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1010,7 +1010,7 @@ _mesa_ast_process_interface_block(YYLTYPE *locp,
>                             "an instance name are not allowed");
>     }
>  
> -   uint64_t interface_type_mask;
> +   ast_type_qualifier::bitset_t interface_type_mask;
>     struct ast_type_qualifier temp_type_qualifier;
>  
>     /* Get a bitmask containing only the in/out/uniform/buffer
> @@ -1029,7 +1029,7 @@ _mesa_ast_process_interface_block(YYLTYPE *locp,
>      * production rule guarantees that only one bit will be set (and
>      * it will be in/out/uniform).
>      */
> -   uint64_t block_interface_qualifier = q.flags.i;
> +   ast_type_qualifier::bitset_t block_interface_qualifier = q.flags.i;
>  
>     block->default_layout.flags.i |= block_interface_qualifier;
>  
>