[Mesa-dev] [PATCH] [RFC] r600g: rework handling of the constants

[Tom Stellard]

On Thu, Dec 20, 2012 at 04:47:51PM +0400, Vadim Girlin wrote:
> Pack kc_bank, const_index and chan into the load.const operand, unpack
> them in r600_alu_from_byte_stream and use to override sel, kc_bank and chan
> for the constants.
> 
> Expected operand value is (((512 + (kc_bank << 12) + const_index) << 2) + chan).
> ---
>  src/gallium/drivers/r600/r600_llvm.c   |  6 +++++-
>  src/gallium/drivers/r600/r600_shader.c | 33 ++++++++++++++++++++++++++-------
>  2 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/r600_llvm.c b/src/gallium/drivers/r600/r600_llvm.c
> index 17d362c..110ba64 100644
> --- a/src/gallium/drivers/r600/r600_llvm.c
> +++ b/src/gallium/drivers/r600/r600_llvm.c
> @@ -25,8 +25,12 @@ static LLVMValueRef llvm_fetch_const(
>  	enum tgsi_opcode_type type,
>  	unsigned swizzle)
>  {
> +	/* load.const's operand currently goes through the compiler unmodified,
> +	 * encoding expected by the bytestream reader is
> +	 * ((512 + (4096 * kc_bank) + const_index)*4 + chan)
> +	 */
>  	LLVMValueRef idx = lp_build_const_int32(bld_base->base.gallivm,
> -			radeon_llvm_reg_index_soa(reg->Register.Index, swizzle));
> +			radeon_llvm_reg_index_soa(reg->Register.Index + 512, swizzle));
>  	LLVMValueRef cval = build_intrinsic(bld_base->base.gallivm->builder,
>  		"llvm.AMDGPU.load.const", bld_base->base.elem_type,
>  		&idx, 1, LLVMReadNoneAttribute);

While we are changing this, I think we should replace the load.const intrinsic
with a load from the constant address space.  You can pass the encoded value as
the pointer.  We do something very similar with USER_SGPRs in radeonsi, so you can
look at use_sgpr() in radeonsi_shader.c and also SITargetLowering::LowerLOAD() in
SIISelLowering.cpp as an example of how to do this.

> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index bcd43f1..5b97088 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -299,15 +299,21 @@ static unsigned r600_src_from_byte_stream(unsigned char * bytes,
>  static unsigned r600_alu_from_byte_stream(struct r600_shader_ctx *ctx,
>  				unsigned char * bytes, unsigned bytes_read)
>  {
> -	unsigned src_idx;
> +	unsigned src_idx, src_num;
>  	struct r600_bytecode_alu alu;
> -	unsigned src_const_reg[3];
> +	unsigned src_use_sel[3];
> +	unsigned src_sel[3] = {};
>  	uint32_t word0, word1;
>  
> +	src_num = bytes[bytes_read++];
> +
>  	memset(&alu, 0, sizeof(alu));
> -	for(src_idx = 0; src_idx < 3; src_idx++) {
> +	for(src_idx = 0; src_idx < src_num; src_idx++) {
>  		unsigned i;
> -		src_const_reg[src_idx] = bytes[bytes_read++];
> +		src_use_sel[src_idx] = bytes[bytes_read++];
> +		for (i = 0; i < 4; i++) {
> +			src_sel[src_idx] |= bytes[bytes_read++] << (i * 8);
> +		}
>  		for (i = 0; i < 4; i++) {
>  			alu.src[src_idx].value |= bytes[bytes_read++] << (i * 8);
>  		}
> @@ -327,9 +333,22 @@ static unsigned r600_alu_from_byte_stream(struct r600_shader_ctx *ctx,
>  		break;
>  	}
>  
> -	for(src_idx = 0; src_idx < 3; src_idx++) {
> -		if (src_const_reg[src_idx])
> -			alu.src[src_idx].sel += 512;
> +	for(src_idx = 0; src_idx < src_num; src_idx++) {
> +		if (src_use_sel[src_idx]) {
> +			unsigned sel = src_sel[src_idx];
> +
> +			alu.src[src_idx].chan = sel & 3;
> +			sel >>= 2;
> +
> +			if (sel>=512) { /* constant */
> +				sel -= 512;
> +				alu.src[src_idx].kc_bank = sel >> 12;
> +				alu.src[src_idx].sel = (sel & 4095) + 512;
> +			}
> +			else {
> +				alu.src[src_idx].sel = sel;
> +			}
> +		}
>  	}

As I mentioned in the other patch, it seems like a lot of this logic could
be moved into the LLVM backend.


-Tom
>  
>  #if HAVE_LLVM < 0x0302
> -- 
> 1.8.0.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev