Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-05-05 Thread Jonathan Gray 
On Fri, Apr 22, 2016 at 11:19:04AM -0700, Chad Versace wrote:
> On 04/22/2016 10:50 AM, Jason Ekstrand wrote:
> > 
> > 
> > On Fri, Apr 22, 2016 at 10:15 AM, Jonathan Gray  > > wrote:
> > 
> > On Fri, Apr 22, 2016 at 05:31:29PM +0100, Emil Velikov wrote:
> >> On 22 April 2016 at 16:08, Jonathan Gray  >> > wrote:
> >>> It is worth noting that the isl code extensively requires
> >>> designated initialisers on anonymous structs.  It isn't clear to
> >>> me when gcc introduced support for this but it isn't in 4.2.
> >>> 
> >> I think it should work for GCC 4.2 with -fms-extensions. We used
> >> to set -std=gnu99 for pre 4.6 which effectively enables it the
> >> extension. Can you double-check ?
> > 
> > The part that sets gnu99 for < gcc 4.6 is still there, using
> > -fms-extensions does not help for these. 
> 
> 
> > At least for the errors you're seeing there, I see two options:  1)
> > Use the isl_extentNd constructor functions in isl.h.  2) Stop making
> > isl_extentNd have anonymous unions.  I'm not sure how much the
> > anonymous unions are really doing for us but I'd like chad to chip in
> > before we throw them out.
> 
> I like the anonymous unions because they make some equations more concise.
> But, they don't provide anymore than that. I'll accept patches to remove
> them, as other people have also commented on their awkwardness.

Mark Kettenis has modified the base compiler to accept designated
initialisers for anonymous types so there is now no need to patch them
out for OpenBSD.

http://marc.info/?l=openbsd-cvs&m=146244074408249&w=2

>  
> > Here's another question: I know BSD doesn't ship gcc newer than 4.2
> > for license issues, but do you have a recent version of clang
> > available?
> 
> GCC 4.2 was released in 2007, and BSD won't upgrade it due to GPLv3.
> I believe it's unreasonable to promise Mesa will indefinitely restrict
> all new code to the feature set provided by a 2007-era GCC. As the years
> roll by, such a promise would become untenable.
> 
> Please try clang.
> 
> >>> Would you accept patches to remove them?
> >> While I cannot comment if they're OK with the idea, there might be 
> >> some confusion on the topic. There is anonymous and named. I
> >> believe developers were against the latter. Examples form [1]
> >> 
> >> struct bar { int i; }; // (1) unnamed, but tagged, ie *not*
> >> anonymous struct { int j; }; // (2) unnamed, but anonymous 
> >> struct { int k; } baz; // (3) named, but not tagged
> >> 
> >> Fwiw it would be great to use the more portable solution. Would
> >> C11 buy us anything ?
> 
> I believe that C11 does fix the problem. It standardized anonymous unions and 
> structs.
> However, if we need to support GCC 4.2, then we can't use C11. In fact, GCC's 
> C11 support
> is still incomplete as 5.0. I've discovered serious bugs as recently as GCC 
> 4.9.
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Jonathan Gray 
On Fri, Apr 22, 2016 at 10:50:33AM -0700, Jason Ekstrand wrote:
> On Fri, Apr 22, 2016 at 10:15 AM, Jonathan Gray  wrote:
>
> > On Fri, Apr 22, 2016 at 05:31:29PM +0100, Emil Velikov wrote:
> > > On 22 April 2016 at 16:08, Jonathan Gray  wrote:
> > > > It is worth noting that the isl code extensively requires designated
> > > > initialisers on anonymous structs.  It isn't clear to me when gcc
> > introduced
> > > > support for this but it isn't in 4.2.
> > > >
> > > I think it should work for GCC 4.2 with -fms-extensions. We used to
> > > set -std=gnu99 for pre 4.6 which effectively enables it the extension.
> > > Can you double-check ?
> >
> > The part that sets gnu99 for < gcc 4.6 is still there, using
> > -fms-extensions
> > does not help for these.
> >
> > libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\"
> > -DPACKAGE_VERSION=\"11.3.0-devel\" "-DPACKAGE_STRING=\"Mesa 11.3.0-devel\""
> > "-DPACKAGE_BUGREPORT=\"
> > https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\"";
> > -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"11.3.0-devel\"
> > -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1
> > -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1
> > -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\"
> > -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1
> > -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1
> > -DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1
> > -DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1
> > -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1
> > -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1
> > -DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1
> > -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_DLADDR=1
> > -DHAVE_CLOCK_GETTIME=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1
> > -DHAVE_SHA1_IN_LIBC=1 -DHAVE_VALGRIND=1 -I. -I/usr/X11R6/include
> > -I/usr/X11R6/include/libdrm -I/usr/local/include/valgrind
> > -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DDEBUG
> > -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF
> > -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM
> > -DHAVE_SHA1 -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING
> > -DHAVE_ALIAS -DHAVE_MINCORE -I../../../include -I../../../src
> > -I../../../src/intel -I../../../src/mapi -I../../../src/mesa
> > -I../../../src/mesa/drivers/dri/common -I../../../src/mesa/drivers/dri/i965
> > -I../../../src/gallium/auxiliary -I../../../src/gallium/include
> > -I../../../src -I../../../src/intel -g -O2 -Wall -std=gnu99
> > -Werror=implicit-function-declaration -Werror=missing-prototypes
> > -fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp
> > -Wno-override-init -DGEN_VERSIONx10=70 -g -O2 -Wall -std=gnu99
> > -Werror=implicit-function-declaration -Werror=missing-prototypes
> > -fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp
> > -MT libisl_gen7_la-isl_gen7.lo -MD -MP -MF
> > .deps/libisl_gen7_la-isl_gen7.Tpo -c isl_gen7.c  -fPIC -DPIC -o
> > .libs/libisl_gen7_la-isl_gen7.o
> > In file included from isl_gen7.h:26,
> >  from isl_gen7.c:24:
> > isl_priv.h: In function 'isl_extent3d_sa_to_el':
> > isl_priv.h:119: error: unknown field 'w' specified in initializer
> > isl_priv.h:119: warning: missing braces around initializer
> > isl_priv.h:119: warning: (near initialization for
> > '(anonymous).')
> > isl_priv.h:120: error: unknown field 'h' specified in initializer
> > isl_priv.h:121: error: unknown field 'd' specified in initializer
> > isl_priv.h: In function 'isl_extent3d_el_to_sa':
> > isl_priv.h:131: error: unknown field 'w' specified in initializer
> > isl_priv.h:131: warning: missing braces around initializer
> > isl_priv.h:131: warning: (near initialization for
> > '(anonymous).')
> > isl_priv.h:132: error: unknown field 'h' specified in initializer
> > isl_priv.h:133: error: unknown field 'd' specified in initializer
> > isl_gen7.c: In function 'gen7_choose_image_alignment_el':
> > isl_gen7.c:391: error: unknown field 'w' specified in initializer
> > isl_gen7.c:391: warning: missing braces around initializer
> > isl_gen7.c:391: warning: (near initialization for
> > '(anonymous).')
> > isl_gen7.c:392: error: unknown field 'h' specified in initializer
> > isl_gen7.c:393: error: unknown field 'd' specified in initializer
> > *** Error 1 in src/intel/isl (Makefile:744 'libisl_gen7_la-isl_gen7.lo')
> >
>
> At least for the errors you're seeing there, I see two options:  1) Use the
> isl_extentNd constructor functions in isl.h.  2) Stop making isl_extentNd
> have anonymous unions.  I'm not sure how much the anonymous unions are
> really doing for us but I'd like chad to chip in before we throw them out.
>
> Here's another question: I know BSD doesn't ship gcc newer than 4.2 for
> license issues, but do you have a recent

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Chad Versace 
On 04/22/2016 10:50 AM, Jason Ekstrand wrote:
> 
> 
> On Fri, Apr 22, 2016 at 10:15 AM, Jonathan Gray  > wrote:
> 
> On Fri, Apr 22, 2016 at 05:31:29PM +0100, Emil Velikov wrote:
>> On 22 April 2016 at 16:08, Jonathan Gray > > wrote:
>>> It is worth noting that the isl code extensively requires
>>> designated initialisers on anonymous structs.  It isn't clear to
>>> me when gcc introduced support for this but it isn't in 4.2.
>>> 
>> I think it should work for GCC 4.2 with -fms-extensions. We used
>> to set -std=gnu99 for pre 4.6 which effectively enables it the
>> extension. Can you double-check ?
> 
> The part that sets gnu99 for < gcc 4.6 is still there, using
> -fms-extensions does not help for these. 


> At least for the errors you're seeing there, I see two options:  1)
> Use the isl_extentNd constructor functions in isl.h.  2) Stop making
> isl_extentNd have anonymous unions.  I'm not sure how much the
> anonymous unions are really doing for us but I'd like chad to chip in
> before we throw them out.

I like the anonymous unions because they make some equations more concise.
But, they don't provide anymore than that. I'll accept patches to remove
them, as other people have also commented on their awkwardness.
 
> Here's another question: I know BSD doesn't ship gcc newer than 4.2
> for license issues, but do you have a recent version of clang
> available?

GCC 4.2 was released in 2007, and BSD won't upgrade it due to GPLv3.
I believe it's unreasonable to promise Mesa will indefinitely restrict
all new code to the feature set provided by a 2007-era GCC. As the years
roll by, such a promise would become untenable.

Please try clang.

>>> Would you accept patches to remove them?
>> While I cannot comment if they're OK with the idea, there might be 
>> some confusion on the topic. There is anonymous and named. I
>> believe developers were against the latter. Examples form [1]
>> 
>> struct bar { int i; }; // (1) unnamed, but tagged, ie *not*
>> anonymous struct { int j; }; // (2) unnamed, but anonymous 
>> struct { int k; } baz; // (3) named, but not tagged
>> 
>> Fwiw it would be great to use the more portable solution. Would
>> C11 buy us anything ?

I believe that C11 does fix the problem. It standardized anonymous unions and 
structs.
However, if we need to support GCC 4.2, then we can't use C11. In fact, GCC's 
C11 support
is still incomplete as 5.0. I've discovered serious bugs as recently as GCC 4.9.

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

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Jason Ekstrand 
On Fri, Apr 22, 2016 at 10:15 AM, Jonathan Gray  wrote:

> On Fri, Apr 22, 2016 at 05:31:29PM +0100, Emil Velikov wrote:
> > On 22 April 2016 at 16:08, Jonathan Gray  wrote:
> > > It is worth noting that the isl code extensively requires designated
> > > initialisers on anonymous structs.  It isn't clear to me when gcc
> introduced
> > > support for this but it isn't in 4.2.
> > >
> > I think it should work for GCC 4.2 with -fms-extensions. We used to
> > set -std=gnu99 for pre 4.6 which effectively enables it the extension.
> > Can you double-check ?
>
> The part that sets gnu99 for < gcc 4.6 is still there, using
> -fms-extensions
> does not help for these.
>
> libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\"
> -DPACKAGE_VERSION=\"11.3.0-devel\" "-DPACKAGE_STRING=\"Mesa 11.3.0-devel\""
> "-DPACKAGE_BUGREPORT=\"
> https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\"";
> -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"11.3.0-devel\"
> -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1
> -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1
> -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\"
> -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1
> -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1
> -DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1
> -DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1
> -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1
> -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1
> -DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1
> -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_DLADDR=1
> -DHAVE_CLOCK_GETTIME=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1
> -DHAVE_SHA1_IN_LIBC=1 -DHAVE_VALGRIND=1 -I. -I/usr/X11R6/include
> -I/usr/X11R6/include/libdrm -I/usr/local/include/valgrind
> -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DDEBUG
> -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF
> -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM
> -DHAVE_SHA1 -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING
> -DHAVE_ALIAS -DHAVE_MINCORE -I../../../include -I../../../src
> -I../../../src/intel -I../../../src/mapi -I../../../src/mesa
> -I../../../src/mesa/drivers/dri/common -I../../../src/mesa/drivers/dri/i965
> -I../../../src/gallium/auxiliary -I../../../src/gallium/include
> -I../../../src -I../../../src/intel -g -O2 -Wall -std=gnu99
> -Werror=implicit-function-declaration -Werror=missing-prototypes
> -fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp
> -Wno-override-init -DGEN_VERSIONx10=70 -g -O2 -Wall -std=gnu99
> -Werror=implicit-function-declaration -Werror=missing-prototypes
> -fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp
> -MT libisl_gen7_la-isl_gen7.lo -MD -MP -MF
> .deps/libisl_gen7_la-isl_gen7.Tpo -c isl_gen7.c  -fPIC -DPIC -o
> .libs/libisl_gen7_la-isl_gen7.o
> In file included from isl_gen7.h:26,
>  from isl_gen7.c:24:
> isl_priv.h: In function 'isl_extent3d_sa_to_el':
> isl_priv.h:119: error: unknown field 'w' specified in initializer
> isl_priv.h:119: warning: missing braces around initializer
> isl_priv.h:119: warning: (near initialization for
> '(anonymous).')
> isl_priv.h:120: error: unknown field 'h' specified in initializer
> isl_priv.h:121: error: unknown field 'd' specified in initializer
> isl_priv.h: In function 'isl_extent3d_el_to_sa':
> isl_priv.h:131: error: unknown field 'w' specified in initializer
> isl_priv.h:131: warning: missing braces around initializer
> isl_priv.h:131: warning: (near initialization for
> '(anonymous).')
> isl_priv.h:132: error: unknown field 'h' specified in initializer
> isl_priv.h:133: error: unknown field 'd' specified in initializer
> isl_gen7.c: In function 'gen7_choose_image_alignment_el':
> isl_gen7.c:391: error: unknown field 'w' specified in initializer
> isl_gen7.c:391: warning: missing braces around initializer
> isl_gen7.c:391: warning: (near initialization for
> '(anonymous).')
> isl_gen7.c:392: error: unknown field 'h' specified in initializer
> isl_gen7.c:393: error: unknown field 'd' specified in initializer
> *** Error 1 in src/intel/isl (Makefile:744 'libisl_gen7_la-isl_gen7.lo')
>

At least for the errors you're seeing there, I see two options:  1) Use the
isl_extentNd constructor functions in isl.h.  2) Stop making isl_extentNd
have anonymous unions.  I'm not sure how much the anonymous unions are
really doing for us but I'd like chad to chip in before we throw them out.

Here's another question: I know BSD doesn't ship gcc newer than 4.2 for
license issues, but do you have a recent version of clang available?
--Jason


> >
> > > Would you accept patches to remove them?
> > While I cannot comment if they're OK with the idea, there might be
> > some confusion on the topic.
> > There is anonymous and named. I bel

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Jonathan Gray 
On Fri, Apr 22, 2016 at 05:31:29PM +0100, Emil Velikov wrote:
> On 22 April 2016 at 16:08, Jonathan Gray  wrote:
> > It is worth noting that the isl code extensively requires designated
> > initialisers on anonymous structs.  It isn't clear to me when gcc introduced
> > support for this but it isn't in 4.2.
> >
> I think it should work for GCC 4.2 with -fms-extensions. We used to
> set -std=gnu99 for pre 4.6 which effectively enables it the extension.
> Can you double-check ?

The part that sets gnu99 for < gcc 4.6 is still there, using -fms-extensions
does not help for these.

libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
-DPACKAGE_VERSION=\"11.3.0-devel\" "-DPACKAGE_STRING=\"Mesa 11.3.0-devel\"" 
"-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\"";
 -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"11.3.0-devel\" 
-DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 
-DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 
-DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" 
-DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1 
-DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1 
-DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1 
-DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1 
-DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 
-DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1 
-DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 
-DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_DLADDR=1 
-DHAVE_CLOCK_GETTIME=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 
-DHAVE_SHA1_IN_LIBC=1 -DHAVE_VALGRIND=1 -I. -I/usr/X11R6/include 
-I/usr/X11R6/include/libdrm -I/usr/local/include/valgrind -D__STDC_LIMIT_MACROS 
-D__STDC_CONSTANT_MACROS -DDEBUG -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM 
-DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF -DHAVE_MKOSTEMP -DHAVE_DLOPEN 
-DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM -DHAVE_SHA1 -DGLX_USE_DRM 
-DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DHAVE_ALIAS -DHAVE_MINCORE 
-I../../../include -I../../../src -I../../../src/intel -I../../../src/mapi 
-I../../../src/mesa -I../../../src/mesa/drivers/dri/common 
-I../../../src/mesa/drivers/dri/i965 -I../../../src/gallium/auxiliary 
-I../../../src/gallium/include -I../../../src -I../../../src/intel -g -O2 -Wall 
-std=gnu99 -Werror=implicit-function-declaration -Werror=missing-prototypes 
-fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp 
-Wno-override-init -DGEN_VERSIONx10=70 -g -O2 -Wall -std=gnu99 
-Werror=implicit-function-declaration -Werror=missing-prototypes 
-fno-strict-aliasing -fno-math-errno -fno-trapping-math -fno-builtin-memcmp -MT 
libisl_gen7_la-isl_gen7.lo -MD -MP -MF .deps/libisl_gen7_la-isl_gen7.Tpo -c 
isl_gen7.c  -fPIC -DPIC -o .libs/libisl_gen7_la-isl_gen7.o
In file included from isl_gen7.h:26,
 from isl_gen7.c:24:
isl_priv.h: In function 'isl_extent3d_sa_to_el':
isl_priv.h:119: error: unknown field 'w' specified in initializer
isl_priv.h:119: warning: missing braces around initializer
isl_priv.h:119: warning: (near initialization for '(anonymous).')
isl_priv.h:120: error: unknown field 'h' specified in initializer
isl_priv.h:121: error: unknown field 'd' specified in initializer
isl_priv.h: In function 'isl_extent3d_el_to_sa':
isl_priv.h:131: error: unknown field 'w' specified in initializer
isl_priv.h:131: warning: missing braces around initializer
isl_priv.h:131: warning: (near initialization for '(anonymous).')
isl_priv.h:132: error: unknown field 'h' specified in initializer
isl_priv.h:133: error: unknown field 'd' specified in initializer
isl_gen7.c: In function 'gen7_choose_image_alignment_el':
isl_gen7.c:391: error: unknown field 'w' specified in initializer
isl_gen7.c:391: warning: missing braces around initializer
isl_gen7.c:391: warning: (near initialization for '(anonymous).')
isl_gen7.c:392: error: unknown field 'h' specified in initializer
isl_gen7.c:393: error: unknown field 'd' specified in initializer
*** Error 1 in src/intel/isl (Makefile:744 'libisl_gen7_la-isl_gen7.lo')

> 
> > Would you accept patches to remove them?
> While I cannot comment if they're OK with the idea, there might be
> some confusion on the topic.
> There is anonymous and named. I believe developers were against the
> latter. Examples form [1]
> 
> struct bar { int i; }; // (1) unnamed, but tagged, ie *not* anonymous
> struct { int j; }; // (2) unnamed, but anonymous
> struct { int k; } baz; // (3) named, but not tagged
> 
> Fwiw it would be great to use the more portable solution. Would C11
> buy us anything ?
> 
> 
> Thanks
> Emil
> 
> [1] 
> http://stackoverflow.com/questions/5063548/initialization-of-anonymous-structures-or-unions-in-c1x
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/me

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Emil Velikov 
On 22 April 2016 at 16:08, Jonathan Gray  wrote:
> It is worth noting that the isl code extensively requires designated
> initialisers on anonymous structs.  It isn't clear to me when gcc introduced
> support for this but it isn't in 4.2.
>
I think it should work for GCC 4.2 with -fms-extensions. We used to
set -std=gnu99 for pre 4.6 which effectively enables it the extension.
Can you double-check ?

> Would you accept patches to remove them?
While I cannot comment if they're OK with the idea, there might be
some confusion on the topic.
There is anonymous and named. I believe developers were against the
latter. Examples form [1]

struct bar { int i; }; // (1) unnamed, but tagged, ie *not* anonymous
struct { int j; }; // (2) unnamed, but anonymous
struct { int k; } baz; // (3) named, but not tagged

Fwiw it would be great to use the more portable solution. Would C11
buy us anything ?


Thanks
Emil

[1] 
http://stackoverflow.com/questions/5063548/initialization-of-anonymous-structures-or-unions-in-c1x
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-22 Thread Jonathan Gray 
It is worth noting that the isl code extensively requires designated
initialisers on anonymous structs.  It isn't clear to me when gcc introduced
support for this but it isn't in 4.2.

Would you accept patches to remove them?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-19 Thread Emil Velikov 
Strictly speaking, if the compiler is GL/DRI agnostic, one could move
it outside of src/mesa/drivers/dri and into src/intel. Although I'm
not sure how much of that is true.

On 16 April 2016 at 20:45, Jason Ekstrand  wrote:

Please add a note in the commit message to warn about Chad's
experience. I'm thinking about, but feel free to tweak/reword

"To avoid build issues, ensure that you're running `make' at the top
level and/or you're executed `make clean' beforehand."

> ---
>  configure.ac  | 3 ++-
>  src/Makefile.am   | 9 +++--
>  src/intel/Makefile.am | 4 
>  src/mesa/drivers/dri/i965/Makefile.am | 7 ++-
>  4 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 9226a0d..e4ce8fe 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2459,7 +2459,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test x$HAVE_SWRAST_DRI 
> = xyes)
>
>  AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
>
> -AM_CONDITIONAL(HAVE_INTEL_DRIVERS, test "x$HAVE_INTEL_VULKAN" = xyes)
> +AM_CONDITIONAL(HAVE_INTEL_DRIVERS, test "x$HAVE_INTEL_VULKAN" = xyes -o \
> +"x$HAVE_I965_DRI" = xyes)
>
>  AM_CONDITIONAL(NEED_RADEON_DRM_WINSYS, test "x$HAVE_GALLIUM_R300" = xyes -o \
>  "x$HAVE_GALLIUM_R600" = xyes -o \
> diff --git a/src/Makefile.am b/src/Makefile.am
> index a0572b7..e9745d4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -24,6 +24,10 @@ SUBDIRS = . gtest util mapi/glapi/gen mapi
>  # include only conditionally ?
>  SUBDIRS += compiler
>
> +if HAVE_INTEL_DRIVERS
> +SUBDIRS += intel
> +endif
> +
>  if NEED_OPENGL_COMMON
>  SUBDIRS += mesa
>  endif
> @@ -56,8 +60,9 @@ EXTRA_DIST = \
>  AM_CFLAGS = $(VISIBILITY_CFLAGS)
>  AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
>
> -if HAVE_INTEL_DRIVERS
> -SUBDIRS += intel
> +# This explicitly comes later because it depends on the i965 compiler
> +if HAVE_INTEL_VULKAN
> +SUBDIRS += intel/vulkan
>  endif
>
Please move this hunk just after mesa, and s/comes later/comes after mesa/


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

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-19 Thread Emil Velikov 
On 19 April 2016 at 20:47, Chad Versace  wrote:
> On Sat 16 Apr 2016, Jason Ekstrand wrote:
>> ---
>>  configure.ac  | 3 ++-
>>  src/Makefile.am   | 9 +++--
>>  src/intel/Makefile.am | 4 
>>  src/mesa/drivers/dri/i965/Makefile.am | 7 ++-
>>  4 files changed, 15 insertions(+), 8 deletions(-)
>
>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index a0572b7..e9745d4 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -24,6 +24,10 @@ SUBDIRS = . gtest util mapi/glapi/gen mapi
>>  # include only conditionally ?
>>  SUBDIRS += compiler
>>
>> +if HAVE_INTEL_DRIVERS
>> +SUBDIRS += intel
>> +endif
>> +
>>  if NEED_OPENGL_COMMON
>>  SUBDIRS += mesa
>>  endif
>> @@ -56,8 +60,9 @@ EXTRA_DIST = \
>>  AM_CFLAGS = $(VISIBILITY_CFLAGS)
>>  AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
>>
>> -if HAVE_INTEL_DRIVERS
>> -SUBDIRS += intel
>> +# This explicitly comes later because it depends on the i965 compiler
>> +if HAVE_INTEL_VULKAN
>> +SUBDIRS += intel/vulkan
>>  endif
>>
>>  AM_CPPFLAGS = \
>> diff --git a/src/intel/Makefile.am b/src/intel/Makefile.am
>> index c3673c6..520602d 100644
>> --- a/src/intel/Makefile.am
>> +++ b/src/intel/Makefile.am
>> @@ -20,7 +20,3 @@
>>  # IN THE SOFTWARE.
>>
>>  SUBDIRS = genxml isl
>> -
>> -if HAVE_INTEL_VULKAN
>> -SUBDIRS += vulkan
>> -endif
>
> Moving the src/intel/vulkan from src/intel/Makefile.am to
> src/Makefile.am makes breaks a long-standing development pattern.
> Post-patch, if I do this:
>
> $ cd $MESA_TOP/src/intel
> $ ed vulkan/anv_cmd_buffer.c
> $ make # SURPRISE! No vulkan code is rebuilt and your driver is weirdly 
> out-of-date.
>
Unless you consider that nothing will ever move (be that a single file
or a whole folder) using this is strongly ill-advised.

One *might* be able to get around it by following the approach use by
xserver (configure keeps track of the static libraries used for the
final foo.so binary) although reportedly that did break something in
their dependency tracking.

> Do you see a way to fix this problem in the short-term (that is, inside
> this patch series and not *eventually*). I don't think the problem is so
> bad that it should void the patch series, but I did want to raise it to
> everyone's attention.
>
Thank you for that. Pretty sure others are abound to run (have ran)
into similar issues.

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

Re: [Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-19 Thread Chad Versace 
On Sat 16 Apr 2016, Jason Ekstrand wrote:
> ---
>  configure.ac  | 3 ++-
>  src/Makefile.am   | 9 +++--
>  src/intel/Makefile.am | 4 
>  src/mesa/drivers/dri/i965/Makefile.am | 7 ++-
>  4 files changed, 15 insertions(+), 8 deletions(-)


> diff --git a/src/Makefile.am b/src/Makefile.am
> index a0572b7..e9745d4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -24,6 +24,10 @@ SUBDIRS = . gtest util mapi/glapi/gen mapi
>  # include only conditionally ?
>  SUBDIRS += compiler
>  
> +if HAVE_INTEL_DRIVERS
> +SUBDIRS += intel
> +endif
> +
>  if NEED_OPENGL_COMMON
>  SUBDIRS += mesa
>  endif
> @@ -56,8 +60,9 @@ EXTRA_DIST = \
>  AM_CFLAGS = $(VISIBILITY_CFLAGS)
>  AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
>  
> -if HAVE_INTEL_DRIVERS
> -SUBDIRS += intel
> +# This explicitly comes later because it depends on the i965 compiler
> +if HAVE_INTEL_VULKAN
> +SUBDIRS += intel/vulkan
>  endif
>  
>  AM_CPPFLAGS = \
> diff --git a/src/intel/Makefile.am b/src/intel/Makefile.am
> index c3673c6..520602d 100644
> --- a/src/intel/Makefile.am
> +++ b/src/intel/Makefile.am
> @@ -20,7 +20,3 @@
>  # IN THE SOFTWARE.
>  
>  SUBDIRS = genxml isl
> -
> -if HAVE_INTEL_VULKAN
> -SUBDIRS += vulkan
> -endif

Moving the src/intel/vulkan from src/intel/Makefile.am to
src/Makefile.am makes breaks a long-standing development pattern.
Post-patch, if I do this:

$ cd $MESA_TOP/src/intel
$ ed vulkan/anv_cmd_buffer.c
$ make # SURPRISE! No vulkan code is rebuilt and your driver is weirdly 
out-of-date.

Do you see a way to fix this problem in the short-term (that is, inside
this patch series and not *eventually*). I don't think the problem is so
bad that it should void the patch series, but I did want to raise it to
everyone's attention.

And it's now LUNCH TIME. I'm walking to your cube to get FOOD.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 01/13] i965: Add a dependency on libisl

2016-04-16 Thread Jason Ekstrand 
---
 configure.ac  | 3 ++-
 src/Makefile.am   | 9 +++--
 src/intel/Makefile.am | 4 
 src/mesa/drivers/dri/i965/Makefile.am | 7 ++-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9226a0d..e4ce8fe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2459,7 +2459,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test x$HAVE_SWRAST_DRI = 
xyes)
 
 AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
 
-AM_CONDITIONAL(HAVE_INTEL_DRIVERS, test "x$HAVE_INTEL_VULKAN" = xyes)
+AM_CONDITIONAL(HAVE_INTEL_DRIVERS, test "x$HAVE_INTEL_VULKAN" = xyes -o \
+"x$HAVE_I965_DRI" = xyes)
 
 AM_CONDITIONAL(NEED_RADEON_DRM_WINSYS, test "x$HAVE_GALLIUM_R300" = xyes -o \
 "x$HAVE_GALLIUM_R600" = xyes -o \
diff --git a/src/Makefile.am b/src/Makefile.am
index a0572b7..e9745d4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,6 +24,10 @@ SUBDIRS = . gtest util mapi/glapi/gen mapi
 # include only conditionally ?
 SUBDIRS += compiler
 
+if HAVE_INTEL_DRIVERS
+SUBDIRS += intel
+endif
+
 if NEED_OPENGL_COMMON
 SUBDIRS += mesa
 endif
@@ -56,8 +60,9 @@ EXTRA_DIST = \
 AM_CFLAGS = $(VISIBILITY_CFLAGS)
 AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
 
-if HAVE_INTEL_DRIVERS
-SUBDIRS += intel
+# This explicitly comes later because it depends on the i965 compiler
+if HAVE_INTEL_VULKAN
+SUBDIRS += intel/vulkan
 endif
 
 AM_CPPFLAGS = \
diff --git a/src/intel/Makefile.am b/src/intel/Makefile.am
index c3673c6..520602d 100644
--- a/src/intel/Makefile.am
+++ b/src/intel/Makefile.am
@@ -20,7 +20,3 @@
 # IN THE SOFTWARE.
 
 SUBDIRS = genxml isl
-
-if HAVE_INTEL_VULKAN
-SUBDIRS += vulkan
-endif
diff --git a/src/mesa/drivers/dri/i965/Makefile.am 
b/src/mesa/drivers/dri/i965/Makefile.am
index a41c830..1ed85af 100644
--- a/src/mesa/drivers/dri/i965/Makefile.am
+++ b/src/mesa/drivers/dri/i965/Makefile.am
@@ -34,6 +34,7 @@ AM_CFLAGS = \
-I$(top_srcdir)/src/mesa/drivers/dri/intel/server \
-I$(top_srcdir)/src/gtest/include \
-I$(top_srcdir)/src/compiler/nir \
+   -I$(top_srcdir)/src/intel \
-I$(top_builddir)/src/compiler/nir \
-I$(top_builddir)/src/mesa/drivers/dri/common \
$(DEFINES) \
@@ -48,13 +49,17 @@ brw_nir_trig_workarounds.c: brw_nir_trig_workarounds.py 
$(top_srcdir)/src/compil
 
 noinst_LTLIBRARIES = libi965_dri.la libi965_compiler.la
 libi965_dri_la_SOURCES = $(i965_FILES)
-libi965_dri_la_LIBADD = libi965_compiler.la $(INTEL_LIBS)
+libi965_dri_la_LIBADD = \
+   $(top_builddir)/src/intel/isl/libisl.la \
+   libi965_compiler.la \
+   $(INTEL_LIBS)
 
 libi965_compiler_la_SOURCES = $(i965_compiler_FILES)
 
 TEST_LIBS = \
libi965_compiler.la \
 ../../../libmesa.la \
+   $(top_builddir)/src/intel/isl/libisl.la \
$(PTHREAD_LIBS) \
$(DLOPEN_LIBS) \
../common/libdri_test_stubs.la
-- 
2.5.0.400.gff86faf

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