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

Submitted by Francisco Jerez on Feb. 25, 2018, 8:12 p.m.

Details

Message ID 87r2p86d6x.fsf@riseup.net
State New
Headers show
Series "Implement latest version of EXT_shader_framebuffer_fetch." ( rev: 3 ) in Mesa

Not browsing as part of any series.

Commit Message

Francisco Jerez Feb. 25, 2018, 8:12 p.m.
Roland Scheidegger <sroland@vmware.com> writes:

> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>> Roland Scheidegger <sroland@vmware.com> 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 &b, 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) <built-in>
>     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
>>>
>>> [...]
>>

Patch hide | download patch | download mbox

From 14d35896450e6d5495c440a3b9440689a91724a1 Mon Sep 17 00:00:00 2001
From: Francisco Jerez <currojerez@riseup.net>
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 b/src/util/bitset.h
index 7bb5f3c83cf..b4c21520238 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=(int x)                                          \
+      {                                                         \
+         const T c = {{ (BITSET_WORD)x }};                      \
+         return *this = c;                                      \
+      }                                                         \
+                                                                \
       friend bool                                               \
       operator==(const T &b, const T &c)                        \
       {                                                         \
@@ -180,6 +170,19 @@  __bitset_next_set(unsigned i, BITSET_WORD *tmp,
          return !(b == c);                                      \
       }                                                         \
                                                                 \
+      friend bool                                               \
+      operator==(const T &b, int x)                             \
+      {                                                         \
+         const T c = {{ (BITSET_WORD)x }};                      \
+         return b == c;                                         \
+      }                                                         \
+                                                                \
+      friend bool                                               \
+      operator!=(const T &b, int x)                             \
+      {                                                         \
+         return !(b == x);                                      \
+      }                                                         \
+                                                                \
       friend T                                                  \
       operator~(const T &b)                                     \
       {                                                         \
-- 
2.16.1


Comments

Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
> Roland Scheidegger <sroland@vmware.com> writes:
> 
>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>> Roland Scheidegger <sroland@vmware.com> 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 &b, 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) <built-in>
>>     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 <sroland@vmware.com>


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

On 25.02.2018 22:12, Francisco Jerez wrote:
> Roland Scheidegger <sroland@vmware.com> writes:
> 
>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>> Roland Scheidegger <sroland@vmware.com> 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 &b, 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) <built-in>
>>      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.t.tamminen@intel.com>


	- 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
>
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 <george.kyriazis@intel.com<mailto:george.kyriazis@intel.com>>


On Feb 25, 2018, at 6:52 PM, Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> wrote:

Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> writes:

Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> 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 &b, 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) <built-in>
   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 <sroland@vmware.com<mailto:sroland@vmware.com>>







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<mailto:mesa-dev@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
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" <george.kyriazis@intel.com> 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 <george.kyriazis@intel.com<mailto:george.kyriazis@intel.com>>
>
> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> wrote:
>
> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> writes:
>
> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> 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 &b, 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) <built-in>
>    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 <sroland@vmware.com<mailto:sroland@vmware.com>>
>
>
>
>
>
>
> 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<mailto:mesa-dev@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Do you care enough to give me a reviewed-by so I could land it right
away?

Roland Scheidegger <sroland@vmware.com> 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" <george.kyriazis@intel.com> 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 <george.kyriazis@intel.com<mailto:george.kyriazis@intel.com>>
>>>
>>> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> wrote:
>>>
>>> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
>>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> writes:
>>>
>>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> 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 &b, 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) <built-in>
>>>    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 <sroland@vmware.com<mailto:sroland@vmware.com>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> 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<mailto:mesa-dev@lists.freedesktop.org>
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
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" <george.kyriazis@intel.com> 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 <george.kyriazis@intel.com<mailto:george.kyriazis@intel.com>>
>>
>> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> wrote:
>>
>> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> writes:
>>
>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> 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 &b, 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) <built-in>
>>    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 <sroland@vmware.com<mailto:sroland@vmware.com>>
>>
>>
>>
>>
>>
>>
>> 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<mailto:mesa-dev@lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Not my area of expertise, but sure.
Reviewed-by: Roland Scheidegger <sroland@vmware.com>


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 <sroland@vmware.com> 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" <george.kyriazis@intel.com> 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 <george.kyriazis@intel.com<mailto:george.kyriazis@intel.com>>
>>>>
>>>> On Feb 25, 2018, at 6:52 PM, Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> wrote:
>>>>
>>>> Am 25.02.2018 um 21:12 schrieb Francisco Jerez:
>>>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> writes:
>>>>
>>>> Am 25.02.2018 um 03:35 schrieb Francisco Jerez:
>>>> Roland Scheidegger <sroland@vmware.com<mailto:sroland@vmware.com>> 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 &b, 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) <built-in>
>>>>    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 <sroland@vmware.com<mailto:sroland@vmware.com>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> 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<mailto:mesa-dev@lists.freedesktop.org>
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev