| Message ID | 20170321050037.GH651@largo.jsg.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 20, 2017 at 10:00 PM, Jonathan Gray <jsg@jsg.id.au> wrote: > On Tue, Mar 21, 2017 at 08:28:22AM +1100, Timothy Arceri wrote: >> >> >> On 21/03/17 06:39, Emil Velikov wrote: >> > On 20 March 2017 at 18:30, Matt Turner <mattst88@gmail.com> wrote: >> > > On Mon, Mar 20, 2017 at 6:55 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> > > > Seems like we ended up all over the place, so let me try afresh. >> > > > >> > > > Above all: >> > > > - Saying "I don't care" about your users is arrogant - let us _not_ >> > > > do that, please ? >> > > >> > > Let's be honest, the OpenBSD is subjecting itself to some pretty >> > > arbitrary restrictions caused including Mesa in its core: 10+ year old >> > > GCC, >> > IIRC Brian was using old MinGW GCC, which was one of the blockers - it >> > wasn't OpenBSD to blame here ;-) >> >> Sorry Emil I probably wasn't clear in our discussion. I sent out patches to >> switch to GCC 4.8 last Sept (I believe this was needed by RHEL6) [1]. >> >> Brain jumped in and said "I'm still using the MinGW gcc 4.6 compiler. I'd >> rather not go through the upgrade hassle if I don't have to." >> >> Followed by Jose "We're internally building and shipping Mesa compiled with >> GCC 4.4 (more specifically 4.4.3). >> >> It's fine if you require GCC 4.8 on automake, but please leave support >> for GCC 4.4.x in SCons." >> >> By this point I got bored and moved on. But OpenBSDs GCC is a fork with >> various features backported, from what I understand Mesa would not build on >> a real GCC 4.2 release and we should not be using it as a min version. IMO >> if OpenBSD want to maintain a GCC fork they can handle a patch to downgrade >> the min GCC version. >> >> I believe Jonathan would like us to stick with 4.2 as min but is prepared to >> deal with it if we move on. > > I would like to see Mesa test features it uses in configure rather than > arbitary versions that are what a certain linux distribution ships with. > The zlib change for instance didn't reference any specific problems with > older versions or interfaces required from newer versions. How can we reasonably do that? In the context of the patch you inlined -- what would make us believe designated initializers aren't available in some version of GCC and we should test for it? They're just a C99 feature AFAIK. And what would we do if we check and they're not available? Presumably you're not advocating for #ifdef'ing and having two pieces of the same code.
On Tue, Mar 21, 2017 at 04:00:37PM +1100, Jonathan Gray wrote: > On Tue, Mar 21, 2017 at 08:28:22AM +1100, Timothy Arceri wrote: > > > > > > On 21/03/17 06:39, Emil Velikov wrote: > > > On 20 March 2017 at 18:30, Matt Turner <mattst88@gmail.com> wrote: > > > > On Mon, Mar 20, 2017 at 6:55 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > > > > > Seems like we ended up all over the place, so let me try afresh. > > > > > > > > > > Above all: > > > > > - Saying "I don't care" about your users is arrogant - let us _not_ > > > > > do that, please ? > > > > > > > > Let's be honest, the OpenBSD is subjecting itself to some pretty > > > > arbitrary restrictions caused including Mesa in its core: 10+ year old > > > > GCC, > > > IIRC Brian was using old MinGW GCC, which was one of the blockers - it > > > wasn't OpenBSD to blame here ;-) > > > > Sorry Emil I probably wasn't clear in our discussion. I sent out patches to > > switch to GCC 4.8 last Sept (I believe this was needed by RHEL6) [1]. > > > > Brain jumped in and said "I'm still using the MinGW gcc 4.6 compiler. I'd > > rather not go through the upgrade hassle if I don't have to." > > > > Followed by Jose "We're internally building and shipping Mesa compiled with > > GCC 4.4 (more specifically 4.4.3). > > > > It's fine if you require GCC 4.8 on automake, but please leave support > > for GCC 4.4.x in SCons." > > > > By this point I got bored and moved on. But OpenBSDs GCC is a fork with > > various features backported, from what I understand Mesa would not build on > > a real GCC 4.2 release and we should not be using it as a min version. IMO > > if OpenBSD want to maintain a GCC fork they can handle a patch to downgrade > > the min GCC version. > > > > I believe Jonathan would like us to stick with 4.2 as min but is prepared to > > deal with it if we move on. > > I would like to see Mesa test features it uses in configure rather than > arbitary versions that are what a certain linux distribution ships with. > The zlib change for instance didn't reference any specific problems with > older versions or interfaces required from newer versions. > > We have one platform using clang/lld now (arm64) and are likely to move > others in future where possible. libtool has to be patched and the Mesa > configure script regenerated to make this work or Mesa won't build due > to libtool.m4 looking for specific strings in ld -v produced by bfd > binutils or gold... > > And yes if you change the configure script to check for a newer version > I'll revert it locally like I did with the zlib one. > > As I get the impression no one cares about patches for older GCC I've > not being sending them to the list, ie And to be clear this is code in Mesa violating the C++ standard according to clang++ --std=c++14 -pedantic. compiler/brw_vec4_gs_visitor.cpp:589:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_POINTS] =_3DPRIM_POINTLIST, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:590:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_LINES] = _3DPRIM_LINELIST, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:591:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_LINE_LOOP] = _3DPRIM_LINELOOP, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:592:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_LINE_STRIP] = _3DPRIM_LINESTRIP, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:593:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_TRIANGLES] = _3DPRIM_TRILIST, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:594:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_TRIANGLE_STRIP] = _3DPRIM_TRISTRIP, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:595:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_TRIANGLE_FAN] = _3DPRIM_TRIFAN, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:596:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_QUADS] = _3DPRIM_QUADLIST, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:597:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_QUAD_STRIP] = _3DPRIM_QUADSTRIP, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:598:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_POLYGON] = _3DPRIM_POLYGON, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:599:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_LINES_ADJACENCY] = _3DPRIM_LINELIST_ADJ, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:600:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_LINE_STRIP_ADJACENCY] = _3DPRIM_LINESTRIP_ADJ, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:601:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_TRIANGLES_ADJACENCY] = _3DPRIM_TRILIST_ADJ, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compiler/brw_vec4_gs_visitor.cpp:602:4: warning: designated initializers are a C99 feature [-Wc99-extensions] [GL_TRIANGLE_STRIP_ADJACENCY] = _3DPRIM_TRISTRIP_ADJ, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
On 21 March 2017 at 05:00, Jonathan Gray <jsg@jsg.id.au> wrote: > On Tue, Mar 21, 2017 at 08:28:22AM +1100, Timothy Arceri wrote: >> >> >> On 21/03/17 06:39, Emil Velikov wrote: >> > On 20 March 2017 at 18:30, Matt Turner <mattst88@gmail.com> wrote: >> > > On Mon, Mar 20, 2017 at 6:55 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> > > > Seems like we ended up all over the place, so let me try afresh. >> > > > >> > > > Above all: >> > > > - Saying "I don't care" about your users is arrogant - let us _not_ >> > > > do that, please ? >> > > >> > > Let's be honest, the OpenBSD is subjecting itself to some pretty >> > > arbitrary restrictions caused including Mesa in its core: 10+ year old >> > > GCC, >> > IIRC Brian was using old MinGW GCC, which was one of the blockers - it >> > wasn't OpenBSD to blame here ;-) >> >> Sorry Emil I probably wasn't clear in our discussion. I sent out patches to >> switch to GCC 4.8 last Sept (I believe this was needed by RHEL6) [1]. >> >> Brain jumped in and said "I'm still using the MinGW gcc 4.6 compiler. I'd >> rather not go through the upgrade hassle if I don't have to." >> >> Followed by Jose "We're internally building and shipping Mesa compiled with >> GCC 4.4 (more specifically 4.4.3). >> >> It's fine if you require GCC 4.8 on automake, but please leave support >> for GCC 4.4.x in SCons." >> >> By this point I got bored and moved on. But OpenBSDs GCC is a fork with >> various features backported, from what I understand Mesa would not build on >> a real GCC 4.2 release and we should not be using it as a min version. IMO >> if OpenBSD want to maintain a GCC fork they can handle a patch to downgrade >> the min GCC version. >> >> I believe Jonathan would like us to stick with 4.2 as min but is prepared to >> deal with it if we move on. > > I would like to see Mesa test features it uses in configure rather than > arbitary versions that are what a certain linux distribution ships with. > The zlib change for instance didn't reference any specific problems with > older versions or interfaces required from newer versions. > > We have one platform using clang/lld now (arm64) and are likely to move > others in future where possible. libtool has to be patched and the Mesa > configure script regenerated to make this work or Mesa won't build due > to libtool.m4 looking for specific strings in ld -v produced by bfd > binutils or gold... > > And yes if you change the configure script to check for a newer version > I'll revert it locally like I did with the zlib one. > > As I get the impression no one cares about patches for older GCC I've > not being sending them to the list, ie > > commit d3d340d6026e516cc405a2eb1d925a7a7a467480 > Author: Jonathan Gray <jsg@jsg.id.au> > Date: Thu Mar 16 00:30:07 2017 +1100 > > i965: don't use designated array initialisation > > Don't use a form of designated array initialisation that breaks gcc 4.2.1. > Jonathan, I think you meant to say: Using C99 designated initializers is not allowed in C++ code, thus the compiler may warn or even fail. > > Signed-off-by: Jonathan Gray <jsg@jsg.id.au> > > diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp b/src/intel/compiler/brw_vec4_gs_visitor.cpp > index 4a8b5be30e..e7a502306e 100644 > --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp > +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp > @@ -585,23 +585,6 @@ vec4_gs_visitor::gs_end_primitive() > emit(OR(dst_reg(this->control_data_bits), this->control_data_bits, mask)); > } > > -static const GLuint gl_prim_to_hw_prim[GL_TRIANGLE_STRIP_ADJACENCY+1] = { We already have C helper get_hw_prim_for_gl_prim that we can use instead. I'll send a patch in a minute. Thanks Emil
diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp b/src/intel/compiler/brw_vec4_gs_visitor.cpp index 4a8b5be30e..e7a502306e 100644 --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp @@ -585,23 +585,6 @@ vec4_gs_visitor::gs_end_primitive() emit(OR(dst_reg(this->control_data_bits), this->control_data_bits, mask)); } -static const GLuint gl_prim_to_hw_prim[GL_TRIANGLE_STRIP_ADJACENCY+1] = { - [GL_POINTS] =_3DPRIM_POINTLIST, - [GL_LINES] = _3DPRIM_LINELIST, - [GL_LINE_LOOP] = _3DPRIM_LINELOOP, - [GL_LINE_STRIP] = _3DPRIM_LINESTRIP, - [GL_TRIANGLES] = _3DPRIM_TRILIST, - [GL_TRIANGLE_STRIP] = _3DPRIM_TRISTRIP, - [GL_TRIANGLE_FAN] = _3DPRIM_TRIFAN, - [GL_QUADS] = _3DPRIM_QUADLIST, - [GL_QUAD_STRIP] = _3DPRIM_QUADSTRIP, - [GL_POLYGON] = _3DPRIM_POLYGON, - [GL_LINES_ADJACENCY] = _3DPRIM_LINELIST_ADJ, - [GL_LINE_STRIP_ADJACENCY] = _3DPRIM_LINESTRIP_ADJ, - [GL_TRIANGLES_ADJACENCY] = _3DPRIM_TRILIST_ADJ, - [GL_TRIANGLE_STRIP_ADJACENCY] = _3DPRIM_TRISTRIP_ADJ, -}; - extern "C" const unsigned * brw_compile_gs(const struct brw_compiler *compiler, void *log_data, void *mem_ctx, @@ -814,9 +797,51 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data, else prog_data->base.urb_entry_size = ALIGN(output_size_bytes, 128) / 128; - assert(shader->info->gs.output_primitive < ARRAY_SIZE(gl_prim_to_hw_prim)); - prog_data->output_topology = - gl_prim_to_hw_prim[shader->info->gs.output_primitive]; + assert(shader->info->gs.output_primitive < (GL_TRIANGLE_STRIP_ADJACENCY+1)); + switch (shader->info->gs.output_primitive) { + case GL_POINTS: + prog_data->output_topology =_3DPRIM_POINTLIST; + break; + case GL_LINES: + prog_data->output_topology = _3DPRIM_LINELIST; + break; + case GL_LINE_LOOP: + prog_data->output_topology = _3DPRIM_LINELOOP; + break; + case GL_LINE_STRIP: + prog_data->output_topology = _3DPRIM_LINESTRIP; + break; + case GL_TRIANGLES: + prog_data->output_topology = _3DPRIM_TRILIST; + break; + case GL_TRIANGLE_STRIP: + prog_data->output_topology = _3DPRIM_TRISTRIP; + break; + case GL_TRIANGLE_FAN: + prog_data->output_topology = _3DPRIM_TRIFAN; + break; + case GL_QUADS: + prog_data->output_topology = _3DPRIM_QUADLIST; + break; + case GL_QUAD_STRIP: + prog_data->output_topology = _3DPRIM_QUADSTRIP; + break; + case GL_POLYGON: + prog_data->output_topology = _3DPRIM_POLYGON; + break; + case GL_LINES_ADJACENCY: + prog_data->output_topology = _3DPRIM_LINELIST_ADJ; + break; + case GL_LINE_STRIP_ADJACENCY: + prog_data->output_topology = _3DPRIM_LINESTRIP_ADJ; + break; + case GL_TRIANGLES_ADJACENCY: + prog_data->output_topology = _3DPRIM_TRILIST_ADJ; + break; + case GL_TRIANGLE_STRIP_ADJACENCY: + prog_data->output_topology = _3DPRIM_TRISTRIP_ADJ; + break; + } prog_data->vertices_in = shader->info->gs.vertices_in;