On Fri, Oct 9, 2015 at 8:07 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 9 October 2015 at 23:27, Rob Clark <robdcl...@gmail.com> wrote: >> On Fri, Oct 9, 2015 at 5:57 PM, Emil Velikov <emil.l.veli...@gmail.com> >> wrote: >>> On 9 October 2015 at 21:31, Rob Clark <robdcl...@gmail.com> wrote: >>>> From: Rob Clark <robcl...@freedesktop.org> >>>> >>>> Add missing enum to gl_system_value_name() and move VARYING_SLOT_MAX / >>>> FRAG_RESULT_MAX / etc into shader_enums.h as suggested by Emil. >>>> >>>> Reported-by: Emil Velikov <emil.l.veli...@gmail.com> >>>> Signed-off-by: Rob Clark <robcl...@freedesktop.org> >>>> --- >>>> Note: punted on the STATIC_ASSERT() thing for now.. kinda wanted some- >>>> think more like tgsi_strings_check() where we check everything once on >>>> startup, so you don't have to trigger something that calls the various >>>> xyz_name() to realize something is out of sync. >>>> >>> I'm confused - isn't static_assert a compile thing ? >> >> oh, heh, good point.. >> >>>> src/glsl/nir/shader_enums.c | 1 + >>>> src/glsl/nir/shader_enums.h | 7 +++++++ >>>> src/mesa/main/mtypes.h | 5 ----- >>>> 3 files changed, 8 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/src/glsl/nir/shader_enums.c b/src/glsl/nir/shader_enums.c >>>> index 3722475..7fcbe81 100644 >>>> --- a/src/glsl/nir/shader_enums.c >>>> +++ b/src/glsl/nir/shader_enums.c >>>> @@ -169,6 +169,7 @@ const char * gl_system_value_name(gl_system_value >>>> sysval) >>>> ENUM(SYSTEM_VALUE_TESS_LEVEL_INNER), >>>> ENUM(SYSTEM_VALUE_LOCAL_INVOCATION_ID), >>>> ENUM(SYSTEM_VALUE_WORK_GROUP_ID), >>>> + ENUM(SYSTEM_VALUE_NUM_WORK_GROUPS), >>>> ENUM(SYSTEM_VALUE_VERTEX_CNT), >>>> }; >>>> return NAME(sysval); >>>> diff --git a/src/glsl/nir/shader_enums.h b/src/glsl/nir/shader_enums.h >>>> index 2a5d2c5..77638ba 100644 >>>> --- a/src/glsl/nir/shader_enums.h >>>> +++ b/src/glsl/nir/shader_enums.h >>>> @@ -233,6 +233,11 @@ typedef enum >>>> VARYING_SLOT_VAR31, >>>> } gl_varying_slot; >>>> >>>> + >>>> +#define VARYING_SLOT_MAX (VARYING_SLOT_VAR0 + MAX_VARYING) >>> I'd keep this and FRAG_RESULT_MAX defined as FOO + 1. Otherwise we'll >>> end in funny spaghetti of header dependencies due to the extra two >>> macros. >> >> not quite sure, but it sounds like you are asking to open-code >> MAX_VARYINGS? I don't think that is needed since that seems to >> basically be a global config.h type thing (I mean mesa/main/config.h, >> not autoconf one) >> >> There isn't really *supposed* to be a VARYING_SLOT_VAR(n-1) (ignoring >> the fact that I added them simply to get nice string values to print >> out in nir_print) >> > I was thinking that #define VARYING_SLOT_MAX (VARYING_SLOT_VAR + 1) > /*VARYING_SLOT_VAR(n) */ is the preferred way. > Hmm can you please point me in the right direction, as to why having > VARYING_SLOT_VAR(n-1) is a bad idea ?
A few of the enums are meant to have their last element be interpreted as xyz0 + n (where n < #define MAX_xyz from config.h) I guess technically the max isn't something that is likely to change frequently but if we ignore the MAX_xyz from config.h that means we have two places to update if the max was ever increased. >>>> +#define VARYING_SLOT_PATCH0 (VARYING_SLOT_MAX) >>>> +#define VARYING_SLOT_TESS_MAX (VARYING_SLOT_PATCH0 + MAX_VARYING) >>>> + >>> As there are defined 'on top'/'after' of the existing enum perhaps we >>> should keep them alongside their PRIM_MAX...UNKNOWN brethren ? >> >> PRIM_MAX is a different thing, not related to anything that is already >> in shader_enums.. >> > I had in mind the relationship wrt the respective enums/macros, rather > than their meaning. Although with the _MAX above in mind, I'm likely > missing something. prim == gl draw primitive (ie. tris/tristrip/quads/points/etc).. not something that is internal to mesa or in shader_enums.h.. BR, -R > Thanks > Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
