[Mesa-dev] [PATCH 1/3] r600g: avoid unnecessary shader exports v2
Alex Deucher
alexdeucher at gmail.com
Tue Jun 26 10:22:54 PDT 2012
On Tue, Jun 26, 2012 at 12:21 PM, <j.glisse at gmail.com> wrote:
> From: Vadim Girlin <vadimgirlin at gmail.com>
>
> In some cases TGSI shader has more color outputs than the number of CBs,
> so it seems we need to limit the number of color exports. This requires
> different shader variants depending on the nr_cbufs, but on the other hand
> we are doing less exports, which are very costly.
>
> v2: fix various piglit regressions
>
> Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>
> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
For the series:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> src/gallium/drivers/r600/evergreen_state.c | 10 +++-------
> src/gallium/drivers/r600/r600_shader.c | 25 ++++++++++++++++++++++---
> src/gallium/drivers/r600/r600_shader.h | 7 ++++++-
> src/gallium/drivers/r600/r600_state.c | 2 ++
> src/gallium/drivers/r600/r600_state_common.c | 4 ++--
> 5 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index b618ca8..3fe95e1 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -2641,18 +2641,14 @@ void evergreen_pipe_shader_ps(struct pipe_context *ctx, struct r600_pipe_shader
> db_shader_control |= S_02880C_KILL_ENABLE(1);
>
> exports_ps = 0;
> - num_cout = 0;
> for (i = 0; i < rshader->noutput; i++) {
> if (rshader->output[i].name == TGSI_SEMANTIC_POSITION ||
> rshader->output[i].name == TGSI_SEMANTIC_STENCIL)
> exports_ps |= 1;
> - else if (rshader->output[i].name == TGSI_SEMANTIC_COLOR) {
> - if (rshader->fs_write_all)
> - num_cout = rshader->nr_cbufs;
> - else
> - num_cout++;
> - }
> }
> +
> + num_cout = rshader->nr_ps_color_exports;
> +
> exports_ps |= S_02884C_EXPORT_COLORS(num_cout);
> if (!exports_ps) {
> /* always at least export 1 component per pixel */
> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index d294084..37914eb 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -801,6 +801,12 @@ static int tgsi_declaration(struct r600_shader_ctx *ctx)
> ctx->cv_output = i;
> break;
> }
> + } else if (ctx->type == TGSI_PROCESSOR_FRAGMENT) {
> + switch (d->Semantic.Name) {
> + case TGSI_SEMANTIC_COLOR:
> + ctx->shader->nr_ps_max_color_exports++;
> + break;
> + }
> }
> break;
> case TGSI_FILE_CONSTANT:
> @@ -1153,8 +1159,10 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> ctx.colors_used = 0;
> ctx.clip_vertex_write = 0;
>
> + shader->nr_ps_color_exports = 0;
> + shader->nr_ps_max_color_exports = 0;
> +
> shader->two_side = (ctx.type == TGSI_PROCESSOR_FRAGMENT) && rctx->two_side;
> - shader->nr_cbufs = rctx->nr_cbufs;
>
> /* register allocations */
> /* Values [0,127] correspond to GPR[0..127].
> @@ -1289,6 +1297,9 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> }
> }
>
> + if (shader->fs_write_all && rctx->chip_class >= EVERGREEN)
> + shader->nr_ps_max_color_exports = 8;
> +
> if (ctx.fragcoord_input >= 0) {
> if (ctx.bc->chip_class == CAYMAN) {
> for (j = 0 ; j < 4; j++) {
> @@ -1528,10 +1539,17 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> break;
> case TGSI_PROCESSOR_FRAGMENT:
> if (shader->output[i].name == TGSI_SEMANTIC_COLOR) {
> + /* never export more colors than the number of CBs */
> + if (next_pixel_base && next_pixel_base >= (rctx->nr_cbufs + rctx->dual_src_blend * 1)) {
> + /* skip export */
> + j--;
> + continue;
> + }
> output[j].array_base = next_pixel_base++;
> output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> + shader->nr_ps_color_exports++;
> if (shader->fs_write_all && (rctx->chip_class >= EVERGREEN)) {
> - for (k = 1; k < shader->nr_cbufs; k++) {
> + for (k = 1; k < rctx->nr_cbufs; k++) {
> j++;
> memset(&output[j], 0, sizeof(struct r600_bytecode_output));
> output[j].gpr = shader->output[i].gpr;
> @@ -1545,6 +1563,7 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> output[j].array_base = next_pixel_base++;
> output[j].inst = BC_INST(ctx.bc, V_SQ_CF_ALLOC_EXPORT_WORD1_SQ_CF_INST_EXPORT);
> output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> + shader->nr_ps_color_exports++;
> }
> }
> } else if (shader->output[i].name == TGSI_SEMANTIC_POSITION) {
> @@ -1595,7 +1614,7 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> }
>
> /* add fake pixel export */
> - if (ctx.type == TGSI_PROCESSOR_FRAGMENT && j == 0) {
> + if (ctx.type == TGSI_PROCESSOR_FRAGMENT && next_pixel_base == 0) {
> memset(&output[j], 0, sizeof(struct r600_bytecode_output));
> output[j].gpr = 0;
> output[j].elem_size = 3;
> diff --git a/src/gallium/drivers/r600/r600_shader.h b/src/gallium/drivers/r600/r600_shader.h
> index 2d35e77..eb0bbf6 100644
> --- a/src/gallium/drivers/r600/r600_shader.h
> +++ b/src/gallium/drivers/r600/r600_shader.h
> @@ -49,7 +49,12 @@ struct r600_shader {
> boolean fs_write_all;
> boolean vs_prohibit_ucps;
> boolean two_side;
> - unsigned nr_cbufs;
> + /* Number of color outputs in the TGSI shader,
> + * sometimes it could be higher than nr_cbufs (bug?).
> + * Also with writes_all property on eg+ it will be set to max CB number */
> + unsigned nr_ps_max_color_exports;
> + /* Real number of ps color exports compiled in the bytecode */
> + unsigned nr_ps_color_exports;
> /* bit n is set if the shader writes gl_ClipDistance[n] */
> unsigned clip_dist_write;
> /* flag is set if the shader writes VS_OUT_MISC_VEC (e.g. for PSIZE) */
> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> index fc75781..b314edc 100644
> --- a/src/gallium/drivers/r600/r600_state.c
> +++ b/src/gallium/drivers/r600/r600_state.c
> @@ -1640,6 +1640,8 @@ static void r600_set_framebuffer_state(struct pipe_context *ctx,
>
> /* build states */
> rctx->have_depth_fb = 0;
> + rctx->nr_cbufs = state->nr_cbufs;
> +
> for (int i = 0; i < state->nr_cbufs; i++) {
> r600_cb(rctx, rstate, state, i);
> }
> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> index 00e1bd0..7755c9e 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -690,8 +690,8 @@ static void r600_update_derived_state(struct r600_context *rctx)
> }
>
> if ((rctx->ps_shader->shader.two_side != rctx->two_side) ||
> - ((rctx->chip_class >= EVERGREEN) && rctx->ps_shader->shader.fs_write_all &&
> - (rctx->ps_shader->shader.nr_cbufs != rctx->nr_cbufs))) {
> + (MIN2(rctx->ps_shader->shader.nr_ps_max_color_exports, rctx->nr_cbufs + rctx->dual_src_blend)
> + != rctx->ps_shader->shader.nr_ps_color_exports)) {
> r600_shader_rebuild(&rctx->context, rctx->ps_shader);
> }
>
> --
> 1.7.10.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list