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

2018-02-27 Thread Roland Scheidegger
Not my area of expertise, but sure.
Reviewed-by: Roland Scheidegger 


Am 27.02.2018 um 20:14 schrieb Francisco Jerez:
> Do you care enough to give me a reviewed-by so I could land it right
> away?
> 
> Roland Scheidegger  writes:
> 
>> Please don't wait any longer. We really want appveyor (and some of our
>> own build systems) going again...
>>
>> Roland
>>
>> Am 27.02.2018 um 19:58 schrieb Francisco Jerez:
>>> Thanks for testing.  I'm going to land the build fix with your
>>> Tested-by's if nobody raises any concerns in the next 24h.
>>>
>>> "Kyriazis, George"  writes:
>>>
 It also fixes the errors that I was getting with gcc 5.4.0 with configure 
 build on ubuntu 16.04.

 Tested-By: George Kyriazis 
 >

 On Feb 25, 2018, at 6:52 PM, Roland Scheidegger 
 > wrote:

 Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
 Roland Scheidegger > writes:

 Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
 Roland Scheidegger > writes:

 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;
^

 Oops...  And the only reason bitset_t has a default constructor was...
 to avoid using another C++11 feature (defaulted member functions).
 Does the attached patch fix the build failure for you?  The cleaner
 alternative would be to define the default constructor of the bitset
 object like 'T() = default', but that would imply dropping support for
 GCC 4.2-4.3 which don't implement the feature...

 FWIW the compile error was happening with gcc 4.8 - I didn't see it with
 gcc 5.4.
 (I don't think at vmware we'd care about anything older than gcc 4.4 at
 least but last time someone wanted to bump gcc requirements there were
 still people requiring gcc 4.2.)

 The patch compiles albeit there's about two dozen warnings like the
 following:
 glsl/ast_type.cpp: In member function 'bool
 ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
 glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
 even though the worst conversion for the first is better than the worst
 conversion for the second: [enabled by default]
return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
   ^
 In file included from glsl/ast.h:31:0,
 from glsl/ast_type.cpp:24:
 ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
 ast_type_qualifier::bitset_t&, unsigned int)
   operator!=(const T , BITSET_WORD x) \
   ^
 glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
DECLARE_BITSET_T(bitset_t, 128);
^
 glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
   ^
 Roland


 Ah, yeah, that's because I didn't provide overloads for signed integer
 types, but it should be harmless since the two candidates have the same
 semantics, and should go away with a C++11-capable compiler.  I think
 the attached patch should shut the warnings on older compilers.

 Yes, that compiles without warnings (with gcc 4.8)
 Tested-by: Roland Scheidegger 
 >






 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

 [...]




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

2018-02-27 Thread Francisco Jerez
Do you care enough to give me a reviewed-by so I could land it right
away?

Roland Scheidegger  writes:

> Please don't wait any longer. We really want appveyor (and some of our
> own build systems) going again...
>
> Roland
>
> Am 27.02.2018 um 19:58 schrieb Francisco Jerez:
>> Thanks for testing.  I'm going to land the build fix with your
>> Tested-by's if nobody raises any concerns in the next 24h.
>> 
>> "Kyriazis, George"  writes:
>> 
>>> It also fixes the errors that I was getting with gcc 5.4.0 with configure 
>>> build on ubuntu 16.04.
>>>
>>> Tested-By: George Kyriazis 
>>> >
>>>
>>> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger 
>>> > wrote:
>>>
>>> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
>>> Roland Scheidegger > writes:
>>>
>>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>> Roland Scheidegger > writes:
>>>
>>> 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;
>>>^
>>>
>>> Oops...  And the only reason bitset_t has a default constructor was...
>>> to avoid using another C++11 feature (defaulted member functions).
>>> Does the attached patch fix the build failure for you?  The cleaner
>>> alternative would be to define the default constructor of the bitset
>>> object like 'T() = default', but that would imply dropping support for
>>> GCC 4.2-4.3 which don't implement the feature...
>>>
>>> FWIW the compile error was happening with gcc 4.8 - I didn't see it with
>>> gcc 5.4.
>>> (I don't think at vmware we'd care about anything older than gcc 4.4 at
>>> least but last time someone wanted to bump gcc requirements there were
>>> still people requiring gcc 4.2.)
>>>
>>> The patch compiles albeit there's about two dozen warnings like the
>>> following:
>>> glsl/ast_type.cpp: In member function 'bool
>>> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
>>> glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
>>> even though the worst conversion for the first is better than the worst
>>> conversion for the second: [enabled by default]
>>>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>>   ^
>>> In file included from glsl/ast.h:31:0,
>>> from glsl/ast_type.cpp:24:
>>> ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
>>> ast_type_qualifier::bitset_t&, unsigned int)
>>>   operator!=(const T , BITSET_WORD x) \
>>>   ^
>>> glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
>>>DECLARE_BITSET_T(bitset_t, 128);
>>>^
>>> glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
>>>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>>   ^
>>> Roland
>>>
>>>
>>> Ah, yeah, that's because I didn't provide overloads for signed integer
>>> types, but it should be harmless since the two candidates have the same
>>> semantics, and should go away with a C++11-capable compiler.  I think
>>> the attached patch should shut the warnings on older compilers.
>>>
>>> Yes, that compiles without warnings (with gcc 4.8)
>>> Tested-by: Roland Scheidegger 
>>> >
>>>
>>>
>>>
>>>
>>>
>>>
>>> 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
>>>
>>> [...]
>>>
>>>
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature

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

2018-02-27 Thread Roland Scheidegger
Please don't wait any longer. We really want appveyor (and some of our
own build systems) going again...

Roland

Am 27.02.2018 um 19:58 schrieb Francisco Jerez:
> Thanks for testing.  I'm going to land the build fix with your
> Tested-by's if nobody raises any concerns in the next 24h.
> 
> "Kyriazis, George"  writes:
> 
>> It also fixes the errors that I was getting with gcc 5.4.0 with configure 
>> build on ubuntu 16.04.
>>
>> Tested-By: George Kyriazis 
>> >
>>
>> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger 
>> > wrote:
>>
>> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
>> Roland Scheidegger > writes:
>>
>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>> Roland Scheidegger > writes:
>>
>> 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;
>>^
>>
>> Oops...  And the only reason bitset_t has a default constructor was...
>> to avoid using another C++11 feature (defaulted member functions).
>> Does the attached patch fix the build failure for you?  The cleaner
>> alternative would be to define the default constructor of the bitset
>> object like 'T() = default', but that would imply dropping support for
>> GCC 4.2-4.3 which don't implement the feature...
>>
>> FWIW the compile error was happening with gcc 4.8 - I didn't see it with
>> gcc 5.4.
>> (I don't think at vmware we'd care about anything older than gcc 4.4 at
>> least but last time someone wanted to bump gcc requirements there were
>> still people requiring gcc 4.2.)
>>
>> The patch compiles albeit there's about two dozen warnings like the
>> following:
>> glsl/ast_type.cpp: In member function 'bool
>> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
>> glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
>> even though the worst conversion for the first is better than the worst
>> conversion for the second: [enabled by default]
>>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>   ^
>> In file included from glsl/ast.h:31:0,
>> from glsl/ast_type.cpp:24:
>> ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
>> ast_type_qualifier::bitset_t&, unsigned int)
>>   operator!=(const T , BITSET_WORD x) \
>>   ^
>> glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
>>DECLARE_BITSET_T(bitset_t, 128);
>>^
>> glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
>>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>   ^
>> Roland
>>
>>
>> Ah, yeah, that's because I didn't provide overloads for signed integer
>> types, but it should be harmless since the two candidates have the same
>> semantics, and should go away with a C++11-capable compiler.  I think
>> the attached patch should shut the warnings on older compilers.
>>
>> Yes, that compiles without warnings (with gcc 4.8)
>> Tested-by: Roland Scheidegger >
>>
>>
>>
>>
>>
>>
>> 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
>>
>> [...]
>>
>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-27 Thread Francisco Jerez
Thanks for testing.  I'm going to land the build fix with your
Tested-by's if nobody raises any concerns in the next 24h.

"Kyriazis, George"  writes:

> It also fixes the errors that I was getting with gcc 5.4.0 with configure 
> build on ubuntu 16.04.
>
> Tested-By: George Kyriazis 
> >
>
> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger 
> > wrote:
>
> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
> Roland Scheidegger > writes:
>
> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
> Roland Scheidegger > writes:
>
> 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;
>^
>
> Oops...  And the only reason bitset_t has a default constructor was...
> to avoid using another C++11 feature (defaulted member functions).
> Does the attached patch fix the build failure for you?  The cleaner
> alternative would be to define the default constructor of the bitset
> object like 'T() = default', but that would imply dropping support for
> GCC 4.2-4.3 which don't implement the feature...
>
> FWIW the compile error was happening with gcc 4.8 - I didn't see it with
> gcc 5.4.
> (I don't think at vmware we'd care about anything older than gcc 4.4 at
> least but last time someone wanted to bump gcc requirements there were
> still people requiring gcc 4.2.)
>
> The patch compiles albeit there's about two dozen warnings like the
> following:
> glsl/ast_type.cpp: In member function 'bool
> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
> glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
> even though the worst conversion for the first is better than the worst
> conversion for the second: [enabled by default]
>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>   ^
> In file included from glsl/ast.h:31:0,
> from glsl/ast_type.cpp:24:
> ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
> ast_type_qualifier::bitset_t&, unsigned int)
>   operator!=(const T , BITSET_WORD x) \
>   ^
> glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
>DECLARE_BITSET_T(bitset_t, 128);
>^
> glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
>return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>   ^
> Roland
>
>
> Ah, yeah, that's because I didn't provide overloads for signed integer
> types, but it should be harmless since the two candidates have the same
> semantics, and should go away with a C++11-capable compiler.  I think
> the attached patch should shut the warnings on older compilers.
>
> Yes, that compiles without warnings (with gcc 4.8)
> Tested-by: Roland Scheidegger >
>
>
>
>
>
>
> 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
>
> [...]
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-26 Thread Kyriazis, George
It also fixes the errors that I was getting with gcc 5.4.0 with configure build 
on ubuntu 16.04.

Tested-By: George Kyriazis 
>

On Feb 25, 2018, at 6:52 PM, Roland Scheidegger 
> wrote:

Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
Roland Scheidegger > writes:

Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
Roland Scheidegger > writes:

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;
   ^

Oops...  And the only reason bitset_t has a default constructor was...
to avoid using another C++11 feature (defaulted member functions).
Does the attached patch fix the build failure for you?  The cleaner
alternative would be to define the default constructor of the bitset
object like 'T() = default', but that would imply dropping support for
GCC 4.2-4.3 which don't implement the feature...

FWIW the compile error was happening with gcc 4.8 - I didn't see it with
gcc 5.4.
(I don't think at vmware we'd care about anything older than gcc 4.4 at
least but last time someone wanted to bump gcc requirements there were
still people requiring gcc 4.2.)

The patch compiles albeit there's about two dozen warnings like the
following:
glsl/ast_type.cpp: In member function 'bool
ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
even though the worst conversion for the first is better than the worst
conversion for the second: [enabled by default]
   return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
  ^
In file included from glsl/ast.h:31:0,
from glsl/ast_type.cpp:24:
../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
ast_type_qualifier::bitset_t&, unsigned int)
  operator!=(const T , BITSET_WORD x) \
  ^
glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
   DECLARE_BITSET_T(bitset_t, 128);
   ^
glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
   return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
  ^
Roland


Ah, yeah, that's because I didn't provide overloads for signed integer
types, but it should be harmless since the two candidates have the same
semantics, and should go away with a C++11-capable compiler.  I think
the attached patch should shut the warnings on older compilers.

Yes, that compiles without warnings (with gcc 4.8)
Tested-by: Roland Scheidegger >






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

[...]



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-26 Thread Eero Tamminen

Hi,

On 25.02.2018 22:12, Francisco Jerez wrote:

Roland Scheidegger  writes:


Am 25.02.2018 um 03:35 schrieb Francisco Jerez:

Roland Scheidegger  writes:


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;
 ^


Oops...  And the only reason bitset_t has a default constructor was...
to avoid using another C++11 feature (defaulted member functions).
Does the attached patch fix the build failure for you?  The cleaner
alternative would be to define the default constructor of the bitset
object like 'T() = default', but that would imply dropping support for
GCC 4.2-4.3 which don't implement the feature...


FWIW the compile error was happening with gcc 4.8 - I didn't see it with
gcc 5.4.


I do.  It's broken with Ubuntu 16.04 gcc:
$ g++ --version
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.6) 5.4.0 20160609


It does compile with gcc 6.3 in Ubuntu 17.04 though.  However, with
that, I get segfault when using INTEL_DEBUG=shader_time with this commit
e.g. with SynMark v7, GpuTest v0.7.  I didn't get segfault when using
day older Mesa git version.



(I don't think at vmware we'd care about anything older than gcc 4.4 at
least but last time someone wanted to bump gcc requirements there were
still people requiring gcc 4.2.)

The patch compiles albeit there's about two dozen warnings like the
following:
glsl/ast_type.cpp: In member function 'bool
ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
even though the worst conversion for the first is better than the worst
conversion for the second: [enabled by default]
 return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
^
In file included from glsl/ast.h:31:0,
  from glsl/ast_type.cpp:24:
../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
ast_type_qualifier::bitset_t&, unsigned int)
operator!=(const T , BITSET_WORD x) \
^
glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
 DECLARE_BITSET_T(bitset_t, 128);
 ^
glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
 return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;


Ah, yeah, that's because I didn't provide overloads for signed integer
types, but it should be harmless since the two candidates have the same
semantics, and should go away with a C++11-capable compiler.  I think
the attached patch should shut the warnings on older compilers.


Yes, it fixed the compilation with gcc 5.4, and INTEL_DEBUG=shader_time
segfaults with gcc 6.3 build.

Tested-by: Eero Tamminen 


- Eero






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

[...]






___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-25 Thread Roland Scheidegger
Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
> Roland Scheidegger  writes:
> 
>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>> Roland Scheidegger  writes:
>>>
 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;
 ^
>>>
>>> Oops...  And the only reason bitset_t has a default constructor was...
>>> to avoid using another C++11 feature (defaulted member functions).
>>> Does the attached patch fix the build failure for you?  The cleaner
>>> alternative would be to define the default constructor of the bitset
>>> object like 'T() = default', but that would imply dropping support for
>>> GCC 4.2-4.3 which don't implement the feature...
>>
>> FWIW the compile error was happening with gcc 4.8 - I didn't see it with
>> gcc 5.4.
>> (I don't think at vmware we'd care about anything older than gcc 4.4 at
>> least but last time someone wanted to bump gcc requirements there were
>> still people requiring gcc 4.2.)
>>
>> The patch compiles albeit there's about two dozen warnings like the
>> following:
>> glsl/ast_type.cpp: In member function 'bool
>> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
>> glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
>> even though the worst conversion for the first is better than the worst
>> conversion for the second: [enabled by default]
>> return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>^
>> In file included from glsl/ast.h:31:0,
>>  from glsl/ast_type.cpp:24:
>> ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
>> ast_type_qualifier::bitset_t&, unsigned int)
>>operator!=(const T , BITSET_WORD x) \
>>^
>> glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
>> DECLARE_BITSET_T(bitset_t, 128);
>> ^
>> glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
>> return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>>^
>> Roland
>>
> 
> Ah, yeah, that's because I didn't provide overloads for signed integer
> types, but it should be harmless since the two candidates have the same
> semantics, and should go away with a C++11-capable compiler.  I think
> the attached patch should shut the warnings on older compilers.

Yes, that compiles without warnings (with gcc 4.8)
Tested-by: Roland Scheidegger 


> 
>>
>>
>>>
 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

 [...]
>>>
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-25 Thread Francisco Jerez
Roland Scheidegger  writes:

> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>> Roland Scheidegger  writes:
>> 
>>> 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;
>>> ^
>> 
>> Oops...  And the only reason bitset_t has a default constructor was...
>> to avoid using another C++11 feature (defaulted member functions).
>> Does the attached patch fix the build failure for you?  The cleaner
>> alternative would be to define the default constructor of the bitset
>> object like 'T() = default', but that would imply dropping support for
>> GCC 4.2-4.3 which don't implement the feature...
>
> FWIW the compile error was happening with gcc 4.8 - I didn't see it with
> gcc 5.4.
> (I don't think at vmware we'd care about anything older than gcc 4.4 at
> least but last time someone wanted to bump gcc requirements there were
> still people requiring gcc 4.2.)
>
> The patch compiles albeit there's about two dozen warnings like the
> following:
> glsl/ast_type.cpp: In member function 'bool
> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
> glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
> even though the worst conversion for the first is better than the worst
> conversion for the second: [enabled by default]
> return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>^
> In file included from glsl/ast.h:31:0,
>  from glsl/ast_type.cpp:24:
> ../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
> ast_type_qualifier::bitset_t&, unsigned int)
>operator!=(const T , BITSET_WORD x) \
>^
> glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
> DECLARE_BITSET_T(bitset_t, 128);
> ^
> glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
> return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
>^
> Roland
>

Ah, yeah, that's because I didn't provide overloads for signed integer
types, but it should be harmless since the two candidates have the same
semantics, and should go away with a C++11-capable compiler.  I think
the attached patch should shut the warnings on older compilers.

>
>
>> 
>>> 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
>>>
>>> [...]
>> 

From 14d35896450e6d5495c440a3b9440689a91724a1 Mon Sep 17 00:00:00 2001
From: Francisco Jerez 
Date: Sat, 24 Feb 2018 18:37:34 -0800
Subject: [PATCH] util/bitset: Make C++ wrapper trivially constructible.

v2: Provide signed integer comparison and assignment operators instead
of BITSET_WORD ones to avoid spurious ambiguity warnings on
comparisons with a signed integer literal.
---
 src/compiler/glsl/ast.h  |  2 --
 src/compiler/glsl/glsl_parser.yy |  1 -
 src/util/bitset.h| 37 -
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index e5e4b572fff..a1ec0d566f4 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -477,8 +477,6 @@ struct ast_type_qualifier {
DECLARE_BITSET_T(bitset_t, 128);
 
union flags {
-  flags() : i(0) {}
-
   struct {
 	 unsigned invariant:1;
  unsigned precise:1;
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index f1986ed0a8a..e5ea41d4dff 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -96,7 +96,6 @@ 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/util/bitset.h 

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

2018-02-25 Thread 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;
>  
> 

___
mesa-dev 

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

2018-02-25 Thread Roland Scheidegger
Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
> Roland Scheidegger  writes:
> 
>> 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;
>> ^
> 
> Oops...  And the only reason bitset_t has a default constructor was...
> to avoid using another C++11 feature (defaulted member functions).
> Does the attached patch fix the build failure for you?  The cleaner
> alternative would be to define the default constructor of the bitset
> object like 'T() = default', but that would imply dropping support for
> GCC 4.2-4.3 which don't implement the feature...

FWIW the compile error was happening with gcc 4.8 - I didn't see it with
gcc 5.4.
(I don't think at vmware we'd care about anything older than gcc 4.4 at
least but last time someone wanted to bump gcc requirements there were
still people requiring gcc 4.2.)

The patch compiles albeit there's about two dozen warnings like the
following:
glsl/ast_type.cpp: In member function 'bool
ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state*) const':
glsl/ast_type.cpp:50:67: warning: ISO C++ says that these are ambiguous,
even though the worst conversion for the first is better than the worst
conversion for the second: [enabled by default]
return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
   ^
In file included from glsl/ast.h:31:0,
 from glsl/ast_type.cpp:24:
../../src/util/bitset.h:181:7: note: candidate 1: bool operator!=(const
ast_type_qualifier::bitset_t&, unsigned int)
   operator!=(const T , BITSET_WORD x) \
   ^
glsl/ast.h:477:4: note: in expansion of macro 'DECLARE_BITSET_T'
DECLARE_BITSET_T(bitset_t, 128);
^
glsl/ast_type.cpp:50:67: note: candidate 2: operator!=(int, int) 
return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
   ^
Roland



> 
>> 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
>>
>> [...]
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-02-25 Thread Francisco Jerez
Roland Scheidegger  writes:

> 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;
> ^

Oops...  And the only reason bitset_t has a default constructor was...
to avoid using another C++11 feature (defaulted member functions).
Does the attached patch fix the build failure for you?  The cleaner
alternative would be to define the default constructor of the bitset
object like 'T() = default', but that would imply dropping support for
GCC 4.2-4.3 which don't implement the feature...

> 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
>
> [...]

From e2b849f7bb167f7083dea09a421d69e922b09f9c Mon Sep 17 00:00:00 2001
From: Francisco Jerez 
Date: Sat, 24 Feb 2018 18:37:34 -0800
Subject: [PATCH] util/bitset: Make C++ wrapper trivially constructible.

---
 src/compiler/glsl/ast.h  |  2 --
 src/compiler/glsl/glsl_parser.yy |  1 -
 src/util/bitset.h| 37 -
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index e5e4b572fff..a1ec0d566f4 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -477,8 +477,6 @@ struct ast_type_qualifier {
DECLARE_BITSET_T(bitset_t, 128);
 
union flags {
-  flags() : i(0) {}
-
   struct {
 	 unsigned invariant:1;
  unsigned precise:1;
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index f1986ed0a8a..e5ea41d4dff 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -96,7 +96,6 @@ 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/util/bitset.h b/src/util/bitset.h
index 7bb5f3c83cf..fee6a13a796 100644
--- a/src/util/bitset.h
+++ b/src/util/bitset.h
@@ -142,23 +142,6 @@ __bitset_next_set(unsigned i, BITSET_WORD *tmp,
  * it as, and N is the number of bits in the bitset.
  */
 #define DECLARE_BITSET_T(T, N) struct T {   \
-  /* XXX - Replace this with an implicitly-defined  \
-   * constructor when support for C++11 defaulted   \
-   * constructors can be assumed (available on GCC 4.4 and  \
-   * later) in order to make the object trivially   \
-   * constructible like a fundamental integer type for  \
-   * convenience.   \
-   */   \
-  T()   \
-  { \
-  } \
-\
-  T(BITSET_WORD x)  \
-  { \
- for (unsigned i = 0; i < BITSET_WORDS(N); i++, x = 0)  \
-words[i] = x;   \
-  } \
-\
   EXPLICIT_CONVERSION   \
   operator bool() const \
   { \
@@ -168,6 +151,13 @@ __bitset_next_set(unsigned i, BITSET_WORD *tmp,
  return false;  \
   } \
 \
+  T &   \
+  operator=(BITSET_WORD x)  \
+  {  

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

2018-02-14 Thread 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;
 
-- 
2.16.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev