[Mesa-dev] [PATCH] radeonsi: fix gl_BaseVertex value in non-indexed draws
Marek Olšák
maraeo at gmail.com
Mon Apr 10 15:05:22 UTC 2017
Hi Nicolai,
I think there is a simpler way to do this. Instead of going through
update_shaders, we can just set some bit in a user data SGPR e.g.
SI_SGPR_VS_STATE_BITS[1] and the vertex shader can clear gl_BaseVertex
based on that bit. There is no performance concern due to additional
instructions, because gl_BaseVertex is unlikely to be used.
Marek
On Sat, Apr 8, 2017 at 12:41 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> gl_BaseVertex is supposed to be 0 in non-indexed draws. Unfortunately, the
> way they're implemented, the VGT always generates indices starting at 0,
> and the VS prolog adds the start index.
>
> There's a VGT_INDX_OFFSET register which causes the VGT to start at a
> driver-defined index. However, this register cannot be written from
> indirect draws.
>
> So fix this unlikely case in the VS prolog.
>
> Fixes a bug in
> KHR-GL45.shader_draw_parameters_tests.ShaderMultiDrawArraysParameters.*
> ---
> src/gallium/drivers/radeonsi/si_pipe.h | 1 +
> src/gallium/drivers/radeonsi/si_shader.c | 17 +++++++++++++++++
> src/gallium/drivers/radeonsi/si_shader.h | 1 +
> src/gallium/drivers/radeonsi/si_state_draw.c | 5 +++++
> src/gallium/drivers/radeonsi/si_state_shaders.c | 2 ++
> 5 files changed, 26 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
> index daf2932..ecf0f41 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -343,20 +343,21 @@ struct si_context {
> int last_sh_base_reg;
> int last_primitive_restart_en;
> int last_restart_index;
> int last_gs_out_prim;
> int last_prim;
> int last_multi_vgt_param;
> int last_rast_prim;
> unsigned last_sc_line_stipple;
> enum pipe_prim_type current_rast_prim; /* primitive type after TES, GS */
> bool gs_tri_strip_adj_fix;
> + bool current_indexed;
>
> /* Scratch buffer */
> struct r600_atom scratch_state;
> struct r600_resource *scratch_buffer;
> unsigned scratch_waves;
> unsigned spi_tmpring_size;
>
> struct r600_resource *compute_scratch_buffer;
>
> /* Emitted derived tessellation state. */
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
> index f5f86f9..e76ee05 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -7871,20 +7871,37 @@ static void si_build_vs_prolog_function(struct si_shader_context *ctx,
> index = LLVMBuildAdd(gallivm->builder,
> LLVMGetParam(func, ctx->param_vertex_id),
> LLVMGetParam(func, SI_SGPR_BASE_VERTEX), "");
> }
>
> index = LLVMBuildBitCast(gallivm->builder, index, ctx->f32, "");
> ret = LLVMBuildInsertValue(gallivm->builder, ret, index,
> num_params++, "");
> }
>
> + /* For DrawArrays(Indirect) and variants, the basevertex loaded into
> + * the SGPR is the 'first' parameter of the draw call. However, the
> + * value returned as gl_BaseVertex to the VS should be 0.
> + */
> + if (key->vs_prolog.states.clear_basevertex) {
> + LLVMValueRef index;
> +
> + index = LLVMBuildAdd(gallivm->builder,
> + LLVMGetParam(func, ctx->param_vertex_id),
> + LLVMGetParam(func, SI_SGPR_BASE_VERTEX), "");
> + index = LLVMBuildBitCast(gallivm->builder, index, ctx->f32, "");
> + ret = LLVMBuildInsertValue(gallivm->builder, ret, index,
> + ctx->param_vertex_id, "");
> + ret = LLVMBuildInsertValue(gallivm->builder, ret, ctx->i32_0,
> + SI_SGPR_BASE_VERTEX, "");
> + }
> +
> si_llvm_build_ret(ctx, ret);
> }
>
> /**
> * Build the vertex shader epilog function. This is also used by the tessellation
> * evaluation shader compiled as VS.
> *
> * The input is PrimitiveID.
> *
> * If PrimitiveID is required by the pixel shader, export it.
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h b/src/gallium/drivers/radeonsi/si_shader.h
> index 17ffc5d..a3fcb42 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -334,20 +334,21 @@ struct si_shader_selector {
> * | | | | |
> * Only VS & PS: VS | -- | -- | -- | -- | PS
> * With GS: ES | -- | -- | GS | VS | PS
> * With Tessel.: LS | HS | VS | -- | -- | PS
> * With both: LS | HS | ES | GS | VS | PS
> */
>
> /* Common VS bits between the shader key and the prolog key. */
> struct si_vs_prolog_bits {
> unsigned instance_divisors[SI_MAX_ATTRIBS];
> + unsigned clear_basevertex:1;
> };
>
> /* Common VS bits between the shader key and the epilog key. */
> struct si_vs_epilog_bits {
> unsigned export_prim_id:1; /* when PS needs it and GS is disabled */
> };
>
> /* Common TCS bits between the shader key and the epilog key. */
> struct si_tcs_epilog_bits {
> unsigned prim_mode:3;
> diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c b/src/gallium/drivers/radeonsi/si_state_draw.c
> index 2c4e371..4549dd2 100644
> --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> @@ -1145,20 +1145,25 @@ void si_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *info)
> else if (sctx->tes_shader.cso)
> rast_prim = sctx->tes_shader.cso->info.properties[TGSI_PROPERTY_TES_PRIM_MODE];
> else
> rast_prim = info->mode;
>
> if (rast_prim != sctx->current_rast_prim) {
> sctx->current_rast_prim = rast_prim;
> sctx->do_update_shaders = true;
> }
>
> + if (info->indexed != sctx->current_indexed) {
> + sctx->current_indexed = info->indexed;
> + sctx->do_update_shaders = true;
> + }
> +
> if (sctx->gs_shader.cso) {
> /* Determine whether the GS triangle strip adjacency fix should
> * be applied. Rotate every other triangle if
> * - triangle strips with adjacency are fed to the GS and
> * - primitive restart is disabled (the rotation doesn't help
> * when the restart occurs after an odd number of triangles).
> */
> bool gs_tri_strip_adj_fix =
> !sctx->tes_shader.cso &&
> info->mode == PIPE_PRIM_TRIANGLE_STRIP_ADJACENCY &&
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index ff4ff01..f821c9d 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1043,20 +1043,22 @@ static inline void si_shader_selector_key(struct pipe_context *ctx,
> if (sctx->tes_shader.cso)
> key->as_ls = 1;
> else if (sctx->gs_shader.cso)
> key->as_es = 1;
> else {
> si_shader_selector_key_hw_vs(sctx, sel, key);
>
> if (sctx->ps_shader.cso && sctx->ps_shader.cso->info.uses_primid)
> key->part.vs.epilog.export_prim_id = 1;
> }
> + if (!sctx->current_indexed && sel->info.uses_basevertex)
> + key->part.vs.prolog.clear_basevertex = 1;
> break;
> case PIPE_SHADER_TESS_CTRL:
> key->part.tcs.epilog.prim_mode =
> sctx->tes_shader.cso->info.properties[TGSI_PROPERTY_TES_PRIM_MODE];
> key->part.tcs.epilog.tes_reads_tess_factors =
> sctx->tes_shader.cso->info.reads_tess_factors;
>
> if (sel == sctx->fixed_func_tcs_shader.cso)
> key->mono.tcs.inputs_to_copy = sctx->vs_shader.cso->outputs_written;
> break;
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list