[Mesa-dev] [RFCv2 01/13] gallium: refactor pipe_shader_state to support multiple IR's

Brian Paul brianp at vmware.com
Tue Nov 10 07:43:18 PST 2015


On 11/08/2015 01:12 PM, Rob Clark wrote:
> The goal is to allow the pipe driver to request something other than
> TGSI, but detect whether what is getting is TGSI vs what it requested.
> The pipe drivers will always have to support TGSI (and convert that into
> whatever it is that they prefer), but in some cases we should be able to
> skip the TGSI intermediate step (such as glsl->nir vs glsl->tgsi->nir).
>
> I think pipe_compute_state should get similar treatment.  Currently,
> afaict, it has one user and one consumer, which has allowed it to be
> sloppy wrt. supporting alternative IR's.
> ---
>   src/gallium/auxiliary/hud/hud_context.c       | 14 +++++++--
>   src/gallium/auxiliary/postprocess/pp_run.c    |  4 ++-
>   src/gallium/auxiliary/tgsi/tgsi_ureg.c        |  6 ++--
>   src/gallium/auxiliary/util/u_simple_shaders.c | 42 +++++++++++++++++++++++----
>   src/gallium/auxiliary/util/u_tests.c          |  7 ++++-
>   src/gallium/include/pipe/p_defines.h          | 12 ++++++--
>   src/gallium/include/pipe/p_state.h            | 20 +++++++++++--
>   7 files changed, 89 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/auxiliary/hud/hud_context.c b/src/gallium/auxiliary/hud/hud_context.c
> index ffe30b8..2344a48 100644
> --- a/src/gallium/auxiliary/hud/hud_context.c
> +++ b/src/gallium/auxiliary/hud/hud_context.c
> @@ -1182,7 +1182,12 @@ hud_create(struct pipe_context *pipe, struct cso_context *cso)
>         };
>
>         struct tgsi_token tokens[1000];
> -      struct pipe_shader_state state = {tokens};
> +      struct pipe_shader_state state;
> +
> +      memset(&state, 0, sizeof(state));

I think you could do:

struct pipe_shader_state state = {0};

> +
> +      state.ir = PIPE_SHADER_IR_TGSI;
> +      state.tokens = tokens;

Or, maybe you could create a little helper function to initialize 
pipe_shader_state:

static inline void
util_init_tgsi_shader_state(struct pipe_shader_state *state,
                             struct tgsi_token *tokens)
{
    state->ir = PIPE_SHADER_IR_TGSI;
    state->tokens = tokens;
    state->llvm = NULL;
    state->native = NULL;
}

That way, if you change the pipe_shader_state to used a named union (or 
no union), you don't have to touch all this code again.


-Brian


>
>         if (!tgsi_text_translate(fragment_shader_text, tokens, Elements(tokens))) {
>            assert(0);
> @@ -1229,7 +1234,12 @@ hud_create(struct pipe_context *pipe, struct cso_context *cso)
>         };
>
>         struct tgsi_token tokens[1000];
> -      struct pipe_shader_state state = {tokens};
> +      struct pipe_shader_state state;
> +
> +      memset(&state, 0, sizeof(state));
> +
> +      state.ir = PIPE_SHADER_IR_TGSI;
> +      state.tokens = tokens;
>
>         if (!tgsi_text_translate(vertex_shader_text, tokens, Elements(tokens))) {
>            assert(0);
> diff --git a/src/gallium/auxiliary/postprocess/pp_run.c b/src/gallium/auxiliary/postprocess/pp_run.c
> index caa2062..6cd2b70 100644
> --- a/src/gallium/auxiliary/postprocess/pp_run.c
> +++ b/src/gallium/auxiliary/postprocess/pp_run.c
> @@ -272,8 +272,10 @@ pp_tgsi_to_state(struct pipe_context *pipe, const char *text, bool isvs,
>         return NULL;
>      }
>
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
>      state.tokens = tokens;
> -   memset(&state.stream_output, 0, sizeof(state.stream_output));
>
>      if (isvs) {
>         ret_state = pipe->create_vs_state(pipe, &state);
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> index f2f5181..6c40bc1 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> @@ -1778,14 +1778,16 @@ void *ureg_create_shader( struct ureg_program *ureg,
>   {
>      struct pipe_shader_state state;
>
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +
>      state.tokens = ureg_finalize(ureg);
>      if(!state.tokens)
>         return NULL;
>
>      if (so)
>         state.stream_output = *so;
> -   else
> -      memset(&state.stream_output, 0, sizeof(state.stream_output));
>
>      switch (ureg->processor) {
>      case TGSI_PROCESSOR_VERTEX:
> diff --git a/src/gallium/auxiliary/util/u_simple_shaders.c b/src/gallium/auxiliary/util/u_simple_shaders.c
> index 6eed337..8be0da9 100644
> --- a/src/gallium/auxiliary/util/u_simple_shaders.c
> +++ b/src/gallium/auxiliary/util/u_simple_shaders.c
> @@ -121,7 +121,12 @@ void *util_make_layered_clear_vertex_shader(struct pipe_context *pipe)
>            "MOV OUT[2], SV[0]\n"
>            "END\n";
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>         assert(0);
> @@ -149,7 +154,12 @@ void *util_make_layered_clear_helper_vertex_shader(struct pipe_context *pipe)
>            "MOV OUT[2].x, SV[0].xxxx\n"
>            "END\n";
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>         assert(0);
> @@ -192,7 +202,12 @@ void *util_make_layered_clear_geometry_shader(struct pipe_context *pipe)
>         "EMIT IMM[0].xxxx\n"
>         "END\n";
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>         assert(0);
> @@ -471,7 +486,12 @@ util_make_fragment_passthrough_shader(struct pipe_context *pipe,
>
>      char text[sizeof(shader_templ)+100];
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      sprintf(text, shader_templ,
>              write_all_cbufs ? "PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1\n" : "",
> @@ -558,7 +578,12 @@ util_make_fs_blit_msaa_gen(struct pipe_context *pipe,
>      const char *type = tgsi_texture_names[tgsi_tex];
>      char text[sizeof(shader_templ)+100];
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
>             tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
> @@ -658,7 +683,12 @@ util_make_fs_blit_msaa_depthstencil(struct pipe_context *pipe,
>      const char *type = tgsi_texture_names[tgsi_tex];
>      char text[sizeof(shader_templ)+100];
>      struct tgsi_token tokens[1000];
> -   struct pipe_shader_state state = {tokens};
> +   struct pipe_shader_state state;
> +
> +   memset(&state, 0, sizeof(state));
> +
> +   state.ir = PIPE_SHADER_IR_TGSI;
> +   state.tokens = tokens;
>
>      assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
>             tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
> diff --git a/src/gallium/auxiliary/util/u_tests.c b/src/gallium/auxiliary/util/u_tests.c
> index 006dfa9..97fa502 100644
> --- a/src/gallium/auxiliary/util/u_tests.c
> +++ b/src/gallium/auxiliary/util/u_tests.c
> @@ -422,7 +422,12 @@ null_constant_buffer(struct pipe_context *ctx)
>               "MOV OUT[0], CONST[0]\n"
>               "END\n";
>         struct tgsi_token tokens[1000];
> -      struct pipe_shader_state state = {tokens};
> +      struct pipe_shader_state state;
> +
> +      memset(&state, 0, sizeof(state));
> +
> +      state.ir = PIPE_SHADER_IR_TGSI;
> +      state.tokens = tokens;
>
>         if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>            puts("Can't compile a fragment shader.");
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index d6f87cc..0a9d98d 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -704,12 +704,20 @@ enum pipe_shader_cap
>
>   /**
>    * Shader intermediate representation.
> + *
> + * Note that if the driver requests something other than TGSI, it must
> + * always be prepared to receive TGSI in addition to its preferred IR.
> + * If the driver requests TGSI as its preferred IR, it will *always*
> + * get TGSI.
> + *
> + * Note that PIPE_SHADER_IR_TGSI should be zero for backwards compat with
> + * state trackers that only understand TGSI.
>    */
>   enum pipe_shader_ir
>   {
> -   PIPE_SHADER_IR_TGSI,
> +   PIPE_SHADER_IR_TGSI = 0,
>      PIPE_SHADER_IR_LLVM,
> -   PIPE_SHADER_IR_NATIVE
> +   PIPE_SHADER_IR_NATIVE,
>   };
>
>   /**
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 6bdf03a..f1c4b49 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -211,10 +211,26 @@ struct pipe_stream_output_info
>      } output[PIPE_MAX_SO_OUTPUTS];
>   };
>
> -
> +/**
> + * The 'ir' parameter identifies whether the shader state contains TGSI
> + * tokens, etc.  If the driver returns 'PIPE_SHADER_IR_TGSI' for the
> + * 'PIPE_SHADER_CAP_PREFERRED_IR' shader param, the ir will *always* be
> + * 'PIPE_SHADER_IR_TGSI' and the tokens ptr will be valid.  If the driver
> + * requests a different 'pipe_shader_ir' type, then it must check the 'ir'
> + * enum to see if it is getting TGSI tokens or its preferred IR.
> + *
> + * TODO pipe_compute_state should probably get similar treatment to handle
> + * multiple IR's in a cleaner way..
> + */
>   struct pipe_shader_state
>   {
> -   const struct tgsi_token *tokens;
> +   enum pipe_shader_ir ir;
> +   /* TODO are anon unions allowed? */
> +   union {
> +      const struct tgsi_token *tokens;
> +      void *llvm;
> +      void *native;
> +   };
>      struct pipe_stream_output_info stream_output;
>   };
>
>



More information about the mesa-dev mailing list