[Mesa-dev] [RFCv3 01/11] gallium: refactor pipe_shader_state to support multiple IR's
Jose Fonseca
jfonseca at vmware.com
Wed Mar 30 13:32:24 UTC 2016
On 31/01/16 20:16, 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.
>
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
> src/gallium/auxiliary/hud/hud_context.c | 9 ++++-----
> src/gallium/auxiliary/postprocess/pp_run.c | 3 +--
> src/gallium/auxiliary/tgsi/tgsi_ureg.c | 4 +---
> src/gallium/auxiliary/util/u_simple_shaders.c | 18 ++++++++++++------
> src/gallium/auxiliary/util/u_tests.c | 3 ++-
> src/gallium/include/pipe/p_defines.h | 12 ++++++++++--
> src/gallium/include/pipe/p_state.h | 26 +++++++++++++++++++++++++-
> 7 files changed, 55 insertions(+), 20 deletions(-)
>
> diff --git a/src/gallium/auxiliary/hud/hud_context.c b/src/gallium/auxiliary/hud/hud_context.c
> index 75afebe..5542342 100644
> --- a/src/gallium/auxiliary/hud/hud_context.c
> +++ b/src/gallium/auxiliary/hud/hud_context.c
> @@ -1218,7 +1218,7 @@ 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;
>
> if (!tgsi_text_translate(fragment_shader_text, tokens, Elements(tokens))) {
> assert(0);
> @@ -1227,7 +1227,7 @@ hud_create(struct pipe_context *pipe, struct cso_context *cso)
> FREE(hud);
> return NULL;
> }
> -
> + pipe_shader_state_from_tgsi(&state, tokens);
> hud->fs_text = pipe->create_fs_state(pipe, &state);
> }
>
> @@ -1265,8 +1265,7 @@ 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;
> if (!tgsi_text_translate(vertex_shader_text, tokens, Elements(tokens))) {
> assert(0);
> pipe_resource_reference(&hud->font.texture, NULL);
> @@ -1274,7 +1273,7 @@ hud_create(struct pipe_context *pipe, struct cso_context *cso)
> FREE(hud);
> return NULL;
> }
> -
> + pipe_shader_state_from_tgsi(&state, tokens);
> hud->vs = pipe->create_vs_state(pipe, &state);
> }
>
> diff --git a/src/gallium/auxiliary/postprocess/pp_run.c b/src/gallium/auxiliary/postprocess/pp_run.c
> index c6c7b88..75f398e 100644
> --- a/src/gallium/auxiliary/postprocess/pp_run.c
> +++ b/src/gallium/auxiliary/postprocess/pp_run.c
> @@ -272,8 +272,7 @@ pp_tgsi_to_state(struct pipe_context *pipe, const char *text, bool isvs,
> return NULL;
> }
>
> - state.tokens = tokens;
> - memset(&state.stream_output, 0, sizeof(state.stream_output));
> + pipe_shader_state_from_tgsi(&state, tokens);
>
> 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 d681150..ee16f5d 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> @@ -1967,14 +1967,12 @@ void *ureg_create_shader( struct ureg_program *ureg,
> {
> struct pipe_shader_state state;
>
> - state.tokens = ureg_finalize(ureg);
> + pipe_shader_state_from_tgsi(&state, 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 7ffb271..03a0f80 100644
> --- a/src/gallium/auxiliary/util/u_simple_shaders.c
> +++ b/src/gallium/auxiliary/util/u_simple_shaders.c
> @@ -121,12 +121,13 @@ 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;
>
> if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> return pipe->create_vs_state(pipe, &state);
> }
>
> @@ -149,12 +150,13 @@ 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;
>
> if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> return pipe->create_vs_state(pipe, &state);
> }
>
> @@ -192,12 +194,13 @@ 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;
>
> if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> return pipe->create_gs_state(pipe, &state);
> }
>
> @@ -471,7 +474,7 @@ 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;
>
> sprintf(text, shader_templ,
> write_all_cbufs ? "PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1\n" : "",
> @@ -482,6 +485,7 @@ util_make_fragment_passthrough_shader(struct pipe_context *pipe,
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> #if 0
> tgsi_dump(state.tokens, 0);
> #endif
> @@ -558,7 +562,7 @@ 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;
>
> assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
> tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
> @@ -571,6 +575,7 @@ util_make_fs_blit_msaa_gen(struct pipe_context *pipe,
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> #if 0
> tgsi_dump(state.tokens, 0);
> #endif
> @@ -658,7 +663,7 @@ 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;
>
> assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
> tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
> @@ -669,6 +674,7 @@ util_make_fs_blit_msaa_depthstencil(struct pipe_context *pipe,
> assert(0);
> return NULL;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> #if 0
> tgsi_dump(state.tokens, 0);
> #endif
> diff --git a/src/gallium/auxiliary/util/u_tests.c b/src/gallium/auxiliary/util/u_tests.c
> index 006dfa9..9ee1a60 100644
> --- a/src/gallium/auxiliary/util/u_tests.c
> +++ b/src/gallium/auxiliary/util/u_tests.c
> @@ -422,13 +422,14 @@ 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;
>
> if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
> puts("Can't compile a fragment shader.");
> util_report_result(FAIL);
> return;
> }
> + pipe_shader_state_from_tgsi(&state, tokens);
> fs = ctx->create_fs_state(ctx, &state);
> cso_set_fragment_shader_handle(cso, fs);
> }
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index 335f4e6..3a1c691 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -717,12 +717,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 2e4d283..31e512c 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -211,13 +211,37 @@ struct pipe_stream_output_info
> } output[PIPE_MAX_SO_OUTPUTS];
> };
>
> -
> +/**
> + * The 'type' 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 'type'
> + * 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
> {
> + enum pipe_shader_ir type;
> + /* TODO move tokens into union. */
> const struct tgsi_token *tokens;
> + union {
> + void *llvm;
> + void *native;
> + } ir;
You could just make this an anonymous union:
struct pipe_shader_state
{
enum pipe_shader_ir type;
union {
const struct tgsi_token *tokens;
void *llvm;
void *native;
};
I think this makes sense at least for a transitory period.
Long term, I'm not sure an unions makes sense at all, especially when
llvm/native etc are typeless. That is, if we're not using the union to
help with type checking/casts, then we might as well just use one opaque
void * pointer for all:
struct pipe_shader_state
{
enum pipe_shader_ir type;
void *ir;
...
and do the casts ourselves.
Otherwise looks sensible to me.
Jose
> struct pipe_stream_output_info stream_output;
> };
>
> +static inline void
> +pipe_shader_state_from_tgsi(struct pipe_shader_state *state,
> + const struct tgsi_token *tokens)
> +{
> + state->type = PIPE_SHADER_IR_TGSI;
> + state->tokens = tokens;
> + memset(&state->stream_output, 0, sizeof(state->stream_output));
> +}
>
> struct pipe_depth_state
> {
>
More information about the mesa-dev
mailing list