[Mesa-dev] Mesa (master): glsl: Switch ast_type_qualifier to a 128-bit bitset.
Roland Scheidegger
sroland at vmware.com
Mon Feb 26 18:00:21 UTC 2018
Am 26.02.2018 um 18:52 schrieb Kyriazis, George:
> Hello Francisco,
>
> This change breaks my ubuntu 16.04 build with gcc 5.4.0. Here’t where it breaks:
>
> CXX glsl/glsl_lexer.lo
> In file included from ../../../src/compiler/glsl/glsl_lexer.ll:27:0:
> ../../../src/compiler/glsl/ast.h:643: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:643:16: note: unrestricted unions only available with -std=c++11 or -std=gnu++11
> In file included from ../../../src/compiler/glsl/glsl_lexer.ll:29:0:
> ../../../src/compiler/glsl/glsl_parser.yy:106:30: error: member ‘ast_type_qualifier YYSTYPE::type_qualifier’ with constructor not allowed in union
> struct ast_type_qualifier type_qualifier;
> ^
> glsl/glsl_lexer.cpp: In function ‘int yy_get_next_buffer(yyscan_t)’:
> glsl/glsl_lexer.cpp:3490:47: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> if ((int) (yyg->yy_n_chars + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_
> ^
> Makefile:2489: recipe for target 'glsl/glsl_lexer.lo’ failed
>
> The following fixes it:
>
> diff --git a/src/compiler/Makefile.am b/src/compiler/Makefile.am
> index fd6811f..76a9a7f 100644
> --- a/src/compiler/Makefile.am
> +++ b/src/compiler/Makefile.am
> @@ -47,6 +47,7 @@ AM_CFLAGS = \
>
> AM_CXXFLAGS = \
> $(VISIBILITY_CXXFLAGS) \
> + $(CXX11_CXXFLAGS) \
> $(MSVC2013_COMPAT_CXXFLAGS)
>
> noinst_LTLIBRARIES = libcompiler.la
> diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
> index 195e440..fb4abc1 100644
> --- a/src/mesa/Makefile.am
> +++ b/src/mesa/Makefile.am
> @@ -118,6 +118,7 @@ AM_CFLAGS = \
> AM_CXXFLAGS = \
> $(LLVM_CFLAGS) \
> $(VISIBILITY_CXXFLAGS) \
> + $(CXX11_CXXFLAGS) \
> $(MSVC2013_COMPAT_CXXFLAGS)
>
> ARCH_LIBS =
>
>
> I am not sure if it is the best place to put this flag, but the basic idea is that -std=c++11 needs to be added to compile those files.
We can't rely on stdc++11 yet.
FWIW a patch has been posted for this problem it just hasn't been
commited yet:
https://lists.freedesktop.org/archives/mesa-dev/2018-February/186790.html
>
> Thanks,
>
> George
>
>
>> On Feb 24, 2018, at 6:34 PM, Francisco Jerez <currojerez at kemper.freedesktop.org> wrote:
>>
>> Module: Mesa
>> Branch: master
>> Commit: ba79a90fb52e1e81fbfb38113e85a56b13497c50
>> URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=ba79a90fb52e1e81fbfb38113e85a56b13497c50
>>
>> Author: Francisco Jerez <currojerez at riseup.net>
>> Date: Mon Feb 12 14:18:15 2018 -0800
>>
>> glsl: Switch ast_type_qualifier to a 128-bit bitset.
>>
>> 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).
>>
>> Reviewed-by: Plamena Manolova <plamena.manolova at intel.com>
>>
>> ---
>>
>> 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 eee2248281..2a38a4b1f7 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() : i(0) {}
>>
>> - 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 19147c7a3e..4faf9602a0 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 81d74e92ce..106417c5c3 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>> @@ -1011,7 +1011,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
>> @@ -1030,7 +1030,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;
>>
>>
>> _______________________________________________
>> mesa-commit mailing list
>> mesa-commit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-commit
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list