[Mesa-dev] [PATCH] radeonsi: reload PS inputs with direct indexing at each use (v2)

Nicolai Hähnle nhaehnle at gmail.com
Wed Sep 14 07:20:13 UTC 2016


Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

On 13.09.2016 22:20, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> The LLVM compiler can CSE interp intrinsics thanks to
> LLVMReadNoneAttribute.
>
> 26011 shaders in 14651 tests
> Totals:
> SGPRS: 1146340 -> 1132676 (-1.19 %)
> VGPRS: 727371 -> 711730 (-2.15 %)
> Spilled SGPRs: 2218 -> 2078 (-6.31 %)
> Spilled VGPRs: 369 -> 369 (0.00 %)
> Scratch VGPRs: 1344 -> 1344 (0.00 %) dwords per thread
> Code Size: 35841268 -> 36009732 (0.47 %) bytes
> LDS: 767 -> 767 (0.00 %) blocks
> Max Waves: 222559 -> 224779 (1.00 %)
> Wait states: 0 -> 0 (0.00 %)
>
> v2: don't call load_input for fragment shaders in emit_declaration
> ---
>  src/gallium/drivers/radeon/radeon_llvm.h           |  6 ++++-
>  .../drivers/radeon/radeon_setup_tgsi_llvm.c        | 30 ++++++++++++++++++----
>  src/gallium/drivers/radeonsi/si_shader.c           | 27 ++++++++-----------
>  3 files changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/radeon_llvm.h b/src/gallium/drivers/radeon/radeon_llvm.h
> index da5b7f5..f508d32 100644
> --- a/src/gallium/drivers/radeon/radeon_llvm.h
> +++ b/src/gallium/drivers/radeon/radeon_llvm.h
> @@ -23,21 +23,23 @@
>   * Authors: Tom Stellard <thomas.stellard at amd.com>
>   *
>   */
>
>  #ifndef RADEON_LLVM_H
>  #define RADEON_LLVM_H
>
>  #include <llvm-c/Core.h>
>  #include "gallivm/lp_bld_init.h"
>  #include "gallivm/lp_bld_tgsi.h"
> +#include "tgsi/tgsi_parse.h"
>
> +#define RADEON_LLVM_MAX_INPUT_SLOTS 32
>  #define RADEON_LLVM_MAX_INPUTS 32 * 4
>  #define RADEON_LLVM_MAX_OUTPUTS 32 * 4
>
>  #define RADEON_LLVM_INITIAL_CF_DEPTH 4
>
>  #define RADEON_LLVM_MAX_SYSTEM_VALUES 4
>
>  struct radeon_llvm_branch {
>  	LLVMBasicBlockRef endif_block;
>  	LLVMBasicBlockRef if_block;
> @@ -55,33 +57,35 @@ struct radeon_llvm_context {
>
>  	/*=== Front end configuration ===*/
>
>  	/* Instructions that are not described by any of the TGSI opcodes. */
>
>  	/** This function is responsible for initilizing the inputs array and will be
>  	  * called once for each input declared in the TGSI shader.
>  	  */
>  	void (*load_input)(struct radeon_llvm_context *,
>  			   unsigned input_index,
> -			   const struct tgsi_full_declaration *decl);
> +			   const struct tgsi_full_declaration *decl,
> +			   LLVMValueRef out[4]);
>
>  	void (*load_system_value)(struct radeon_llvm_context *,
>  				  unsigned index,
>  				  const struct tgsi_full_declaration *decl);
>
>  	void (*declare_memory_region)(struct radeon_llvm_context *,
>  				      const struct tgsi_full_declaration *decl);
>
>  	/** This array contains the input values for the shader.  Typically these
>  	  * values will be in the form of a target intrinsic that will inform the
>  	  * backend how to load the actual inputs to the shader.
>  	  */
> +	struct tgsi_full_declaration input_decls[RADEON_LLVM_MAX_INPUT_SLOTS];
>  	LLVMValueRef inputs[RADEON_LLVM_MAX_INPUTS];
>  	LLVMValueRef outputs[RADEON_LLVM_MAX_OUTPUTS][TGSI_NUM_CHANNELS];
>
>  	/** This pointer is used to contain the temporary values.
>  	  * The amount of temporary used in tgsi can't be bound to a max value and
>  	  * thus we must allocate this array at runtime.
>  	  */
>  	LLVMValueRef *temps;
>  	unsigned temps_count;
>  	LLVMValueRef system_values[RADEON_LLVM_MAX_SYSTEM_VALUES];
> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> index 4643e6d..4fa43cd 100644
> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> @@ -439,28 +439,43 @@ LLVMValueRef radeon_llvm_emit_fetch(struct lp_build_tgsi_context *bld_base,
>  							bld_base->int_bld.zero);
>  			result = LLVMConstInsertElement(result,
>  							bld->immediates[reg->Register.Index][swizzle + 1],
>  							bld_base->int_bld.one);
>  			return LLVMConstBitCast(result, ctype);
>  		} else {
>  			return LLVMConstBitCast(bld->immediates[reg->Register.Index][swizzle], ctype);
>  		}
>  	}
>
> -	case TGSI_FILE_INPUT:
> -		result = ctx->inputs[radeon_llvm_reg_index_soa(reg->Register.Index, swizzle)];
> +	case TGSI_FILE_INPUT: {
> +		unsigned index = reg->Register.Index;
> +		LLVMValueRef input[4];
> +
> +		/* I don't think doing this for vertex shaders is beneficial.
> +		 * For those, we want to make sure the VMEM loads are executed
> +		 * only once. Fragment shaders don't care much, because
> +		 * v_interp instructions are much cheaper than VMEM loads.
> +		 */
> +		if (ctx->soa.bld_base.info->processor == PIPE_SHADER_FRAGMENT)
> +			ctx->load_input(ctx, index, &ctx->input_decls[index], input);
> +		else
> +			memcpy(input, &ctx->inputs[index * 4], sizeof(input));
> +
> +		result = input[swizzle];
> +
>  		if (tgsi_type_is_64bit(type)) {
>  			ptr = result;
> -			ptr2 = ctx->inputs[radeon_llvm_reg_index_soa(reg->Register.Index, swizzle + 1)];
> +			ptr2 = input[swizzle + 1];
>  			return radeon_llvm_emit_fetch_64bit(bld_base, type, ptr, ptr2);
>  		}
>  		break;
> +	}
>
>  	case TGSI_FILE_TEMPORARY:
>  		if (reg->Register.Index >= ctx->temps_count)
>  			return LLVMGetUndef(tgsi2llvmtype(bld_base, type));
>  		ptr = ctx->temps[reg->Register.Index * TGSI_NUM_CHANNELS + swizzle];
>  		if (tgsi_type_is_64bit(type)) {
>  			ptr2 = ctx->temps[reg->Register.Index * TGSI_NUM_CHANNELS + swizzle + 1];
>  			return radeon_llvm_emit_fetch_64bit(bld_base, type,
>  						 LLVMBuildLoad(builder, ptr, ""),
>  						 LLVMBuildLoad(builder, ptr2, ""));
> @@ -619,22 +634,27 @@ static void emit_declaration(struct lp_build_tgsi_context *bld_base,
>  				}
>  				ctx->temps[first * TGSI_NUM_CHANNELS + i] = ptr;
>  			}
>  		}
>  		break;
>  	}
>  	case TGSI_FILE_INPUT:
>  	{
>  		unsigned idx;
>  		for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) {
> -			if (ctx->load_input)
> -				ctx->load_input(ctx, idx, decl);
> +			if (ctx->load_input) {
> +				ctx->input_decls[idx] = *decl;
> +
> +				if (bld_base->info->processor != PIPE_SHADER_FRAGMENT)
> +					ctx->load_input(ctx, idx, decl,
> +							&ctx->inputs[idx * 4]);
> +			}
>  		}
>  	}
>  	break;
>
>  	case TGSI_FILE_SYSTEM_VALUE:
>  	{
>  		unsigned idx;
>  		for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) {
>  			ctx->load_system_value(ctx, idx, decl);
>  		}
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
> index 8432e45..38e2e0e 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -433,21 +433,22 @@ static LLVMValueRef get_instance_index_for_fetch(
>  		result = LLVMBuildUDiv(gallivm->builder, result,
>  				lp_build_const_int32(gallivm, divisor), "");
>
>  	return LLVMBuildAdd(gallivm->builder, result,
>  			    LLVMGetParam(radeon_bld->main_fn, param_start_instance), "");
>  }
>
>  static void declare_input_vs(
>  	struct radeon_llvm_context *radeon_bld,
>  	unsigned input_index,
> -	const struct tgsi_full_declaration *decl)
> +	const struct tgsi_full_declaration *decl,
> +	LLVMValueRef out[4])
>  {
>  	struct lp_build_context *base = &radeon_bld->soa.bld_base.base;
>  	struct gallivm_state *gallivm = base->gallivm;
>  	struct si_shader_context *ctx =
>  		si_shader_context(&radeon_bld->soa.bld_base);
>  	unsigned divisor =
>  		ctx->shader->key.vs.prolog.instance_divisors[input_index];
>
>  	unsigned chan;
>
> @@ -491,25 +492,22 @@ static void declare_input_vs(
>  	args[0] = t_list;
>  	args[1] = attribute_offset;
>  	args[2] = buffer_index;
>  	input = lp_build_intrinsic(gallivm->builder,
>  		"llvm.SI.vs.load.input", ctx->v4f32, args, 3,
>  		LLVMReadNoneAttribute);
>
>  	/* Break up the vec4 into individual components */
>  	for (chan = 0; chan < 4; chan++) {
>  		LLVMValueRef llvm_chan = lp_build_const_int32(gallivm, chan);
> -		/* XXX: Use a helper function for this.  There is one in
> - 		 * tgsi_llvm.c. */
> -		ctx->radeon_bld.inputs[radeon_llvm_reg_index_soa(input_index, chan)] =
> -				LLVMBuildExtractElement(gallivm->builder,
> -				input, llvm_chan, "");
> +		out[chan] = LLVMBuildExtractElement(gallivm->builder,
> +						    input, llvm_chan, "");
>  	}
>  }
>
>  static LLVMValueRef get_primitive_id(struct lp_build_tgsi_context *bld_base,
>  				     unsigned swizzle)
>  {
>  	struct si_shader_context *ctx = si_shader_context(bld_base);
>
>  	if (swizzle > 0)
>  		return bld_base->uint_bld.zero;
> @@ -1456,47 +1454,44 @@ static LLVMValueRef get_interp_param(struct si_shader_context *ctx,
>  	}
>
>  	if (!param)
>  		param = LLVMGetParam(main_fn, interp_param_idx);
>  	return param;
>  }
>
>  static void declare_input_fs(
>  	struct radeon_llvm_context *radeon_bld,
>  	unsigned input_index,
> -	const struct tgsi_full_declaration *decl)
> +	const struct tgsi_full_declaration *decl,
> +	LLVMValueRef out[4])
>  {
>  	struct lp_build_context *base = &radeon_bld->soa.bld_base.base;
>  	struct si_shader_context *ctx =
>  		si_shader_context(&radeon_bld->soa.bld_base);
>  	struct si_shader *shader = ctx->shader;
>  	LLVMValueRef main_fn = radeon_bld->main_fn;
>  	LLVMValueRef interp_param = NULL;
>  	int interp_param_idx;
>
>  	/* Get colors from input VGPRs (set by the prolog). */
>  	if (!ctx->is_monolithic &&
>  	    decl->Semantic.Name == TGSI_SEMANTIC_COLOR) {
>  		unsigned i = decl->Semantic.Index;
>  		unsigned colors_read = shader->selector->info.colors_read;
>  		unsigned mask = colors_read >> (i * 4);
>  		unsigned offset = SI_PARAM_POS_FIXED_PT + 1 +
>  				  (i ? util_bitcount(colors_read & 0xf) : 0);
>
> -		radeon_bld->inputs[radeon_llvm_reg_index_soa(input_index, 0)] =
> -			mask & 0x1 ? LLVMGetParam(main_fn, offset++) : base->undef;
> -		radeon_bld->inputs[radeon_llvm_reg_index_soa(input_index, 1)] =
> -			mask & 0x2 ? LLVMGetParam(main_fn, offset++) : base->undef;
> -		radeon_bld->inputs[radeon_llvm_reg_index_soa(input_index, 2)] =
> -			mask & 0x4 ? LLVMGetParam(main_fn, offset++) : base->undef;
> -		radeon_bld->inputs[radeon_llvm_reg_index_soa(input_index, 3)] =
> -			mask & 0x8 ? LLVMGetParam(main_fn, offset++) : base->undef;
> +		out[0] = mask & 0x1 ? LLVMGetParam(main_fn, offset++) : base->undef;
> +		out[1] = mask & 0x2 ? LLVMGetParam(main_fn, offset++) : base->undef;
> +		out[2] = mask & 0x4 ? LLVMGetParam(main_fn, offset++) : base->undef;
> +		out[3] = mask & 0x8 ? LLVMGetParam(main_fn, offset++) : base->undef;
>  		return;
>  	}
>
>  	interp_param_idx = lookup_interp_param_index(decl->Interp.Interpolate,
>  						     decl->Interp.Location);
>  	if (interp_param_idx == -1)
>  		return;
>  	else if (interp_param_idx) {
>  		interp_param_idx = select_interp_param(ctx,
>  						       interp_param_idx);
> @@ -1506,21 +1501,21 @@ static void declare_input_fs(
>  	if (decl->Semantic.Name == TGSI_SEMANTIC_COLOR &&
>  	    decl->Interp.Interpolate == TGSI_INTERPOLATE_COLOR &&
>  	    ctx->shader->key.ps.prolog.flatshade_colors)
>  		interp_param = NULL; /* load the constant color */
>
>  	interp_fs_input(ctx, input_index, decl->Semantic.Name,
>  			decl->Semantic.Index, shader->selector->info.num_inputs,
>  			shader->selector->info.colors_read, interp_param,
>  			LLVMGetParam(main_fn, SI_PARAM_PRIM_MASK),
>  			LLVMGetParam(main_fn, SI_PARAM_FRONT_FACE),
> -			&radeon_bld->inputs[radeon_llvm_reg_index_soa(input_index, 0)]);
> +			&out[0]);
>  }
>
>  static LLVMValueRef get_sample_id(struct radeon_llvm_context *radeon_bld)
>  {
>  	return unpack_param(si_shader_context(&radeon_bld->soa.bld_base),
>  			    SI_PARAM_ANCILLARY, 8, 4);
>  }
>
>  /**
>   * Set range metadata on an instruction.  This can only be used on load and
>


More information about the mesa-dev mailing list