[Mesa-dev] [PATCH] glsl: couple shader_enums cleanups

[Emil Velikov]

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 ?

>  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.

> +#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 ?

Thanks for sticking with my suggestion.
Emil