[Mesa-dev] [PATCH] glsl: couple shader_enums cleanups
emil.l.velikov at gmail.com
Fri Oct 9 17:07:50 PDT 2015
On 9 October 2015 at 23:27, Rob Clark <robdclark at gmail.com> wrote:
> On Fri, Oct 9, 2015 at 5:57 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 9 October 2015 at 21:31, Rob Clark <robdclark at gmail.com> wrote:
>>> From: Rob Clark <robclark at 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.velikov at gmail.com>
>>> Signed-off-by: Rob Clark <robclark at 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_NUM_WORK_GROUPS),
>>> 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
>>> } 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
> 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 ?
>>> +#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
More information about the mesa-dev