[Mesa-dev] [PATCH 7/7] radeonsi: use ac_build_buffer_load for shader buffer loads

[Nicolai Hähnle]

Patches 3-5 & 7:

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

And patch #6 as well, if my question on it is clarified.

Cheers,
Nicolai


On 25.05.2017 19:04, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> and document why we can't use SMEM yet.
> ---
>   src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 43 +++++++++++------------
>   1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
> index e2be9e5..c53bde1 100644
> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
> @@ -407,46 +407,45 @@ static unsigned get_store_intr_attribs(bool writeonly_memory)
>   	return writeonly_memory && HAVE_LLVM >= 0x0400 ?
>   				  LP_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
>   				  LP_FUNC_ATTR_WRITEONLY;
>   }
>   
>   static void load_emit_buffer(struct si_shader_context *ctx,
>   			     struct lp_build_emit_data *emit_data,
>   			     bool can_speculate)
>   {
>   	const struct tgsi_full_instruction *inst = emit_data->inst;
> -	struct gallivm_state *gallivm = &ctx->gallivm;
> -	LLVMBuilderRef builder = gallivm->builder;
>   	uint writemask = inst->Dst[0].Register.WriteMask;
>   	uint count = util_last_bit(writemask);
> -	const char *intrinsic_name;
> -	LLVMTypeRef dst_type;
> +	LLVMValueRef *args = emit_data->args;
>   
> -	switch (count) {
> -	case 1:
> -		intrinsic_name = "llvm.amdgcn.buffer.load.f32";
> -		dst_type = ctx->f32;
> -		break;
> -	case 2:
> -		intrinsic_name = "llvm.amdgcn.buffer.load.v2f32";
> -		dst_type = LLVMVectorType(ctx->f32, 2);
> -		break;
> -	default: // 3 & 4
> -		intrinsic_name = "llvm.amdgcn.buffer.load.v4f32";
> -		dst_type = ctx->v4f32;
> -		count = 4;
> -	}
> +	/* Don't use SMEM for shader buffer loads, because LLVM doesn't
> +	 * select SMEM for SI.load.const with a non-constant offset, and
> +	 * constant offsets practically don't exist with shader buffers.
> +	 *
> +	 * Also, SI.load.const doesn't use inst_offset when it's lowered
> +	 * to VMEM, so we just end up with more VALU instructions in the end
> +	 * and no benefit.
> +	 *
> +	 * TODO: Remove this line once LLVM can select SMEM with a non-constant
> +	 *       offset, and can derive inst_offset when VMEM is selected.
> +	 *       After that, si_memory_barrier should invalidate sL1 for shader
> +	 *       buffers.
> +	 */
>   
> -	emit_data->output[emit_data->chan] = lp_build_intrinsic(
> -			builder, intrinsic_name, dst_type,
> -			emit_data->args, emit_data->arg_count,
> -			get_load_intr_attribs(can_speculate));
> +	assert(LLVMConstIntGetZExtValue(args[1]) == 0); /* vindex */
> +	emit_data->output[emit_data->chan] =
> +		ac_build_buffer_load(&ctx->ac, args[0], count, NULL,
> +				     args[2], NULL, 0,
> +				     LLVMConstIntGetZExtValue(args[3]),
> +				     LLVMConstIntGetZExtValue(args[4]),
> +				     can_speculate, false);
>   }
>   
>   static LLVMValueRef get_memory_ptr(struct si_shader_context *ctx,
>                                      const struct tgsi_full_instruction *inst,
>                                      LLVMTypeRef type, int arg)
>   {
>   	struct gallivm_state *gallivm = &ctx->gallivm;
>   	LLVMBuilderRef builder = gallivm->builder;
>   	LLVMValueRef offset, ptr;
>   	int addr_space;
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.