[Mesa-dev] [PATCH] st/mesa: replace _mesa_sysval_to_semantic table with function
Jason Ekstrand
jason at jlekstrand.net
Tue Apr 12 00:04:40 UTC 2016
On Mon, Apr 11, 2016 at 4:31 PM, Roland Scheidegger <sroland at vmware.com>
wrote:
> Am 12.04.2016 um 00:39 schrieb Brian Paul:
> > Instead of using an array indexed by SYSTEM_VALUE_x, just use a
> > switch statement. This fixes a regression caused by inserting new
> > SYSTEM_VALUE_ enums but not updating the mapping to TGSI semantics.
> >
> > v2: fix a few switch statement mistakes for compute-related enums
> > ---
> > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 132
> ++++++++++++++---------------
> > src/mesa/state_tracker/st_glsl_to_tgsi.h | 3 +-
> > src/mesa/state_tracker/st_mesa_to_tgsi.c | 2 +-
> > 3 files changed, 69 insertions(+), 68 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > index b9ab7ae..5f037da 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > @@ -5192,43 +5192,72 @@ struct st_translate {
> > };
> >
> > /** Map Mesa's SYSTEM_VALUE_x to TGSI_SEMANTIC_x */
> > -const unsigned _mesa_sysval_to_semantic[SYSTEM_VALUE_MAX] = {
> > - /* Vertex shader
> > - */
> > - TGSI_SEMANTIC_VERTEXID,
> > - TGSI_SEMANTIC_INSTANCEID,
> > - TGSI_SEMANTIC_VERTEXID_NOBASE,
> > - TGSI_SEMANTIC_BASEVERTEX,
> > - TGSI_SEMANTIC_BASEINSTANCE,
> > - TGSI_SEMANTIC_DRAWID,
> > -
> > - /* Geometry shader
> > - */
> > - TGSI_SEMANTIC_INVOCATIONID,
> > -
> > - /* Fragment shader
> > - */
> > - TGSI_SEMANTIC_POSITION,
> > - TGSI_SEMANTIC_FACE,
> > - TGSI_SEMANTIC_SAMPLEID,
> > - TGSI_SEMANTIC_SAMPLEPOS,
> > - TGSI_SEMANTIC_SAMPLEMASK,
> > - TGSI_SEMANTIC_HELPER_INVOCATION,
> > -
> > - /* Tessellation shaders
> > - */
> > - TGSI_SEMANTIC_TESSCOORD,
> > - TGSI_SEMANTIC_VERTICESIN,
> > - TGSI_SEMANTIC_PRIMID,
> > - TGSI_SEMANTIC_TESSOUTER,
> > - TGSI_SEMANTIC_TESSINNER,
> > +unsigned
> > +_mesa_sysval_to_semantic(unsigned sysval)
> > +{
> > + switch (sysval) {
> > + /* Vertex shader */
> > + case SYSTEM_VALUE_VERTEX_ID:
> > + return TGSI_SEMANTIC_VERTEXID;
> > + case SYSTEM_VALUE_INSTANCE_ID:
> > + return TGSI_SEMANTIC_INSTANCEID;
> > + case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
> > + return TGSI_SEMANTIC_VERTEXID_NOBASE;
> > + case SYSTEM_VALUE_BASE_VERTEX:
> > + return TGSI_SEMANTIC_BASEVERTEX;
> > + case SYSTEM_VALUE_BASE_INSTANCE:
> > + return TGSI_SEMANTIC_BASEINSTANCE;
> > + case SYSTEM_VALUE_DRAW_ID:
> > + return TGSI_SEMANTIC_DRAWID;
> > +
> > + /* Geometry shader */
> > + case SYSTEM_VALUE_INVOCATION_ID:
> > + return TGSI_SEMANTIC_INVOCATIONID;
> > +
> > + /* Fragment shader */
> > + case SYSTEM_VALUE_FRAG_COORD:
> > + return TGSI_SEMANTIC_POSITION;
> > + case SYSTEM_VALUE_FRONT_FACE:
> > + return TGSI_SEMANTIC_FACE;
> > + case SYSTEM_VALUE_SAMPLE_ID:
> > + return TGSI_SEMANTIC_SAMPLEID;
> > + case SYSTEM_VALUE_SAMPLE_POS:
> > + return TGSI_SEMANTIC_SAMPLEPOS;
> > + case SYSTEM_VALUE_SAMPLE_MASK_IN:
> > + return TGSI_SEMANTIC_SAMPLEMASK;
> > + case SYSTEM_VALUE_HELPER_INVOCATION:
> > + return TGSI_SEMANTIC_HELPER_INVOCATION;
> > +
> > + /* Tessellation shader */
> > + case SYSTEM_VALUE_TESS_COORD:
> > + return TGSI_SEMANTIC_TESSCOORD;
> > + case SYSTEM_VALUE_VERTICES_IN:
> > + return TGSI_SEMANTIC_VERTICESIN;
> > + case SYSTEM_VALUE_PRIMITIVE_ID:
> > + return TGSI_SEMANTIC_PRIMID;
> > + case SYSTEM_VALUE_TESS_LEVEL_OUTER:
> > + return TGSI_SEMANTIC_TESSOUTER;
> > + case SYSTEM_VALUE_TESS_LEVEL_INNER:
> > + return TGSI_SEMANTIC_TESSINNER;
> > +
> > + /* Compute shader */
> > + case SYSTEM_VALUE_LOCAL_INVOCATION_ID:
> > + return TGSI_SEMANTIC_THREAD_ID;
> > + case SYSTEM_VALUE_WORK_GROUP_ID:
> > + return TGSI_SEMANTIC_BLOCK_ID;
> > + case SYSTEM_VALUE_NUM_WORK_GROUPS:
> > + return TGSI_SEMANTIC_GRID_SIZE;
> > +
> > + /* Unhandled */
> > + case SYSTEM_VALUE_LOCAL_INVOCATION_INDEX:
> > + case SYSTEM_VALUE_GLOBAL_INVOCATION_ID:
> > + case SYSTEM_VALUE_VERTEX_CNT:
> If you wanted to include all existing values, you've missed the new
> SYSTEM_VALUE_INSTANCE_INDEX here.
>
> (fwiw I find the names confusing - we have:
> SYSTEM_VALUE_VERTEX_ID - includes baseindex
> SYSTEM_VALUE_VERTEX_ID_ZERO_BASE - without baseindex
> SYSTEM_VALUE_INSTANCE_ID - without baseinstance
> SYSTEM_VALUE_INSTANCE_INDEX - includes baseinstance
> And yes I'm aware the inconsistent naming is due to GL's naming
> essentially, but still...)
>
Would you like a patch to rename VERTEX_ID to VERTEX_INDEX and
VERTEX_ID_ZERO_BASE to VERTEX_ID. That's sort-of the convention that was
chosen during the Vulkan discussions to remove confusion. I'd happily
write such a patch. Unfortunately, there would be confusion because
gl_VertexID would be VERTEX_INDEX while gl_InstanceID would be
INSTANCE_INDEX. Fun fun.
--Jason
> Either way, thanks for fixing this,
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>
>
>
>
> > + default:
> > + assert(!"Unexpected SYSTEM_VALUE_ enum");
> > + return TGSI_SEMANTIC_COUNT;
> > + }
> > +}
> >
> > - /* Compute shaders
> > - */
> > - TGSI_SEMANTIC_THREAD_ID,
> > - TGSI_SEMANTIC_BLOCK_ID,
> > - TGSI_SEMANTIC_GRID_SIZE,
> > -};
> >
> > /**
> > * Make note of a branch to a label in the TGSI code.
> > @@ -6000,35 +6029,6 @@ st_translate_program(
> > assert(numInputs <= ARRAY_SIZE(t->inputs));
> > assert(numOutputs <= ARRAY_SIZE(t->outputs));
> >
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_FRONT_FACE] ==
> > - TGSI_SEMANTIC_FACE);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_VERTEX_ID] ==
> > - TGSI_SEMANTIC_VERTEXID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_INSTANCE_ID] ==
> > - TGSI_SEMANTIC_INSTANCEID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_SAMPLE_ID] ==
> > - TGSI_SEMANTIC_SAMPLEID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_SAMPLE_POS] ==
> > - TGSI_SEMANTIC_SAMPLEPOS);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_SAMPLE_MASK_IN] ==
> > - TGSI_SEMANTIC_SAMPLEMASK);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_INVOCATION_ID] ==
> > - TGSI_SEMANTIC_INVOCATIONID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_VERTEX_ID_ZERO_BASE] ==
> > - TGSI_SEMANTIC_VERTEXID_NOBASE);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_BASE_VERTEX] ==
> > - TGSI_SEMANTIC_BASEVERTEX);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_TESS_COORD] ==
> > - TGSI_SEMANTIC_TESSCOORD);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_HELPER_INVOCATION] ==
> > - TGSI_SEMANTIC_HELPER_INVOCATION);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_LOCAL_INVOCATION_ID] ==
> > - TGSI_SEMANTIC_THREAD_ID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_WORK_GROUP_ID] ==
> > - TGSI_SEMANTIC_BLOCK_ID);
> > - assert(_mesa_sysval_to_semantic[SYSTEM_VALUE_NUM_WORK_GROUPS] ==
> > - TGSI_SEMANTIC_GRID_SIZE);
> > -
> > t = CALLOC_STRUCT(st_translate);
> > if (!t) {
> > ret = PIPE_ERROR_OUT_OF_MEMORY;
> > @@ -6215,7 +6215,7 @@ st_translate_program(
> >
> > for (i = 0; sysInputs; i++) {
> > if (sysInputs & (1 << i)) {
> > - unsigned semName = _mesa_sysval_to_semantic[i];
> > + unsigned semName = _mesa_sysval_to_semantic(i);
> >
> > t->systemValues[i] = ureg_DECL_system_value(ureg, semName,
> 0);
> >
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.h
> b/src/mesa/state_tracker/st_glsl_to_tgsi.h
> > index 729295b..774588a 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.h
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.h
> > @@ -63,7 +63,8 @@ st_translate_stream_output_info(struct
> glsl_to_tgsi_visitor *glsl_to_tgsi,
> > const GLuint outputMapping[],
> > struct pipe_stream_output_info *so);
> >
> > -extern const unsigned _mesa_sysval_to_semantic[SYSTEM_VALUE_MAX];
> > +unsigned
> > +_mesa_sysval_to_semantic(unsigned sysval);
> >
> > #ifdef __cplusplus
> > }
> > diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > index 7a686b1..e1c79a5 100644
> > --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > @@ -1074,7 +1074,7 @@ st_translate_mesa_program(
> >
> > for (i = 0; sysInputs; i++) {
> > if (sysInputs & (1 << i)) {
> > - unsigned semName = _mesa_sysval_to_semantic[i];
> > + unsigned semName = _mesa_sysval_to_semantic(i);
> >
> > t->systemValues[i] = ureg_DECL_system_value(ureg, semName,
> 0);
> >
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160411/dff8157d/attachment.html>
More information about the mesa-dev
mailing list