[Mesa-dev] [PATCH] ac/nir: fix user sgpr allocations on gfx9

[Michael Schellenberger Costa]

Hi Dave


Am 19.01.2018 um 00:22 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
>
> The code to decide how to allocate sgprs didn't take into
> account the merged shaders on gfx9.
>
> This updates the code to count the correct number of requires
> sgprs, which means we should be able to enable push const
> inlining easier.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>   src/amd/common/ac_nir_to_llvm.c | 38 +++++++++++++++++++++++++++-----------
>   1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 02a46dab4db..7560b61e99b 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -565,8 +565,21 @@ static bool needs_view_index_sgpr(struct nir_to_llvm_context *ctx,
>   	return false;
>   }
>   
> +static int add_vertex_user_sgprs(const struct ac_shader_info *shader_info)
> +{
> +	int sgpr_count = 0;
> +	sgpr_count += shader_info->vs.has_vertex_buffers ? 2 : 0;
> +	if (shader_info->vs.needs_draw_id) {
> +		sgpr_count += 3;
> +	} else {
> +		sgpr_count += 2;
> +	}
> +	return sgpr_count;
> +}
> +

Wouldn`t this be better written as:

static int add_vertex_user_sgprs(const struct ac_shader_info *shader_info)
{
	int sgpr_count = shader_info->vs.has_vertex_buffers ? 2 : 0;
	sgpr_count += shader_info->vs.needs_draw_id ? 3 : 2;
	return sgpr_count;
}

--Michael

>   static void allocate_user_sgprs(struct nir_to_llvm_context *ctx,
>   				gl_shader_stage stage,
> +				bool has_previous_stage, gl_shader_stage previous_stage,
>   				bool needs_view_index,
>   				struct user_sgpr_info *user_sgpr_info)
>   {
> @@ -589,7 +602,6 @@ static void allocate_user_sgprs(struct nir_to_llvm_context *ctx,
>   		user_sgpr_info->sgpr_count += 2;
>   	}
>   
> -	/* FIXME: fix the number of user sgprs for merged shaders on GFX9 */
>   	switch (stage) {
>   	case MESA_SHADER_COMPUTE:
>   		if (ctx->shader_info->info.cs.uses_grid_size)
> @@ -599,24 +611,28 @@ static void allocate_user_sgprs(struct nir_to_llvm_context *ctx,
>   		user_sgpr_info->sgpr_count += ctx->shader_info->info.ps.needs_sample_positions;
>   		break;
>   	case MESA_SHADER_VERTEX:
> -		if (!ctx->is_gs_copy_shader) {
> -			user_sgpr_info->sgpr_count += ctx->shader_info->info.vs.has_vertex_buffers ? 2 : 0;
> -			if (ctx->shader_info->info.vs.needs_draw_id) {
> -				user_sgpr_info->sgpr_count += 3;
> -			} else {
> -				user_sgpr_info->sgpr_count += 2;
> -			}
> -		}
> +		if (!ctx->is_gs_copy_shader)
> +			user_sgpr_info->sgpr_count += add_vertex_user_sgprs(&ctx->shader_info->info);
>   		if (ctx->options->key.vs.as_ls)
>   			user_sgpr_info->sgpr_count++;
>   		break;
>   	case MESA_SHADER_TESS_CTRL:
> -		user_sgpr_info->sgpr_count += 4;
> +		if (has_previous_stage) {
> +			user_sgpr_info->sgpr_count += add_vertex_user_sgprs(&ctx->shader_info->info);
> +			user_sgpr_info->sgpr_count += 5;
> +		} else
> +			user_sgpr_info->sgpr_count += 4;
>   		break;
>   	case MESA_SHADER_TESS_EVAL:
>   		user_sgpr_info->sgpr_count += 1;
>   		break;
>   	case MESA_SHADER_GEOMETRY:
> +		if (has_previous_stage) {
> +			if (previous_stage == MESA_SHADER_VERTEX)
> +				user_sgpr_info->sgpr_count += add_vertex_user_sgprs(&ctx->shader_info->info);
> +			else if (previous_stage == MESA_SHADER_TESS_EVAL)
> +				user_sgpr_info->sgpr_count += 1;
> +		}
>   		user_sgpr_info->sgpr_count += 2;
>   		break;
>   	default:
> @@ -798,7 +814,7 @@ static void create_function(struct nir_to_llvm_context *ctx,
>   	struct arg_info args = {};
>   	LLVMValueRef desc_sets;
>   	bool needs_view_index = needs_view_index_sgpr(ctx, stage);
> -	allocate_user_sgprs(ctx, stage, needs_view_index, &user_sgpr_info);
> +	allocate_user_sgprs(ctx, stage, has_previous_stage, stage, needs_view_index, &user_sgpr_info);
>   
>   	if (user_sgpr_info.need_ring_offsets && !ctx->options->supports_spill) {
>   		add_arg(&args, ARG_SGPR, const_array(ctx->ac.v4i32, 16),