[Mesa-dev] [PATCH 1/2] r600g: rework handling of the constants
Tom Stellard
tom at stellard.net
Thu Jan 3 07:07:18 PST 2013
On Tue, Jan 01, 2013 at 01:42:34AM +0400, Vadim Girlin wrote:
> On 12/31/2012 10:39 PM, Vincent wrote:
> >Le dimanche 30 décembre 2012 à 10:29 -0500, Tom Stellard a écrit :
> >>On Wed, Dec 26, 2012 at 05:38:27PM +0100, Vincent Lejeune wrote:
> >>> From Vadim Girlin patch
> >>>---
> >>> src/gallium/drivers/r600/r600_llvm.c | 27 +++++++++++----
> >>> src/gallium/drivers/r600/r600_shader.c | 39 ++++++++++++++++------
> >>> .../drivers/radeon/radeon_setup_tgsi_llvm.c | 4 +++
> >>> 3 files changed, 54 insertions(+), 16 deletions(-)
> >>>
> >>>diff --git a/src/gallium/drivers/r600/r600_llvm.c b/src/gallium/drivers/r600/r600_llvm.c
> >>>index 79f6cf0..cae2131 100644
> >>>--- a/src/gallium/drivers/r600/r600_llvm.c
> >>>+++ b/src/gallium/drivers/r600/r600_llvm.c
> >>>@@ -25,12 +25,19 @@ static LLVMValueRef llvm_fetch_const(
> >>> enum tgsi_opcode_type type,
> >>> unsigned swizzle)
> >>> {
> >>>- LLVMValueRef idx = lp_build_const_int32(bld_base->base.gallivm,
> >>>- radeon_llvm_reg_index_soa(reg->Register.Index, swizzle));
> >>>- LLVMValueRef cval = build_intrinsic(bld_base->base.gallivm->builder,
> >>>- "llvm.AMDGPU.load.const", bld_base->base.elem_type,
> >>>- &idx, 1, LLVMReadNoneAttribute);
> >>>-
> >>>+ LLVMValueRef offset[2] = {
> >>>+ LLVMConstInt(LLVMInt64TypeInContext(bld_base->base.gallivm->context), 0, false),
> >>>+ lp_build_const_int32(bld_base->base.gallivm, reg->Register.Index)
> >>>+ };
> >>>+ if (reg->Register.Indirect) {
> >>>+ struct lp_build_tgsi_soa_context *bld = lp_soa_context(bld_base);
> >>>+ LLVMValueRef index = LLVMBuildLoad(bld_base->base.gallivm->builder, bld->addr[reg->Indirect.Index][reg->Indirect.SwizzleX], "");
> >>>+ offset[1] = LLVMBuildAdd(bld_base->base.gallivm->builder, offset[1], index, "");
> >>>+ }
> >>>+ LLVMValueRef const_ptr = LLVMGetFirstGlobal(bld_base->base.gallivm->module);
> >>>+ LLVMValueRef ptr = LLVMBuildGEP(bld_base->base.gallivm->builder, const_ptr, offset, 2, "");
> >>>+ LLVMValueRef cvecval = LLVMBuildLoad(bld_base->base.gallivm->builder, ptr, "");
> >>>+ LLVMValueRef cval = LLVMBuildExtractElement(bld_base->base.gallivm->builder, cvecval, lp_build_const_int32(bld_base->base.gallivm, swizzle), "");
> >>> return bitcast(bld_base, type, cval);
> >>> }
> >>
> >>I thought it would be possible to implement this without using global
> >>addresses by passing the offset as a pointer to the backend and letting
> >>the backend figure out how to translate it into a kcache address.
> >>Can you explain why we are using global variables here?
> >>
> >>-Tom
> >
> >The main drawback from the offset as a pointer solution is that it does
> >not allow to handle multiple const buffers read. R600g already needs to
> >fetch value from non-default const buffer for clip vertex ; it currently
> >appends some alus to llvm backend output but this part would better fit
> >inside the backend IMHO.
>
> Can't you simply encode buffer index as a part of the pointer, e.g.
> pointer = buffer_index << offset + const_index,
> similar to the load_const operand?
>
I don't think this will work well with indirect addressing, since it will be
difficult to extract this information after doing pointer arithmetic. I'm
OK with the current approach, but I think a better solution may be to create a
separate address space for each constant buffer.
-Tom
> >I use global variable because it allows to name pointers and thus to
> >associate a const buffer id with a name (actually "const0" stands for
> >the default const buffer, "const1" is for the second const buffer, and
> >so on).
> >I don't really use the fact it is a global variable inside the backend,
> >in fact any named pointer marked as external could fit (iirc the
> >"external symbol" llvm value could be used too ). Global variables
> >purpose is to model address (function and/or variables) whose value is
> >know at compile time, whereas ExternalSymbol is for address whose value
> >is only know at link time.
> >
> >Vincent
> >
> >
> >>
> >>>
> >>>@@ -449,6 +456,14 @@ LLVMModuleRef r600_tgsi_llvm(
> >>> bld_base->op_actions[TGSI_OPCODE_TXP].emit = llvm_emit_tex;
> >>> bld_base->op_actions[TGSI_OPCODE_CMP].emit = emit_cndlt;
> >>>
> >>>+ // Generate const buffer pointers
> >>>+ for (unsigned i = 0; i < 16; i++) {
> >>>+ char name[8];
> >>>+ sprintf(name, "const%d", i);
> >>>+ LLVMTypeRef type = LLVMArrayType(LLVMVectorType(bld_base->base.elem_type, 4), 1024);
> >>>+ LLVMAddGlobalInAddressSpace(bld_base->base.gallivm->module, type, name, 2);
> >>>+ }
> >>>+
> >>> lp_build_tgsi_llvm(bld_base, tokens);
> >>>
> >>> radeon_llvm_finalize_module(ctx);
> >>>diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> >>>index ac28d22..f8eadd3 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;
> >>>+ }
> >>>+ }
> >>> }
> >>>
> >>> #if HAVE_LLVM < 0x0302
> >>>@@ -503,7 +522,7 @@ static int r600_vtx_from_byte_stream(struct r600_shader_ctx *ctx,
> >>> fprintf(stderr, "Error adding vtx\n");
> >>> }
> >>> /* Use the Texture Cache */
> >>>- ctx->bc->cf_last->inst = EG_V_SQ_CF_WORD1_SQ_CF_INST_TEX;
> >>>+ ctx->bc->cf_last->inst = V_SQ_CF_WORD1_SQ_CF_INST_VTX;
> >>> return bytes_read;
> >>> }
> >>>
> >>>@@ -1239,7 +1258,7 @@ static int r600_shader_from_tgsi(struct r600_screen *rscreen,
> >>> }
> >>>
> >>> #ifdef R600_USE_LLVM
> >>>- if (use_llvm && ctx.info.indirect_files) {
> >>>+ if (use_llvm && ctx.info.indirect_files && (ctx.info.indirect_files & (1 << TGSI_FILE_CONSTANT)) != ctx.info.indirect_files) {
> >>> fprintf(stderr, "Warning: R600 LLVM backend does not support "
> >>> "indirect adressing. Falling back to TGSI "
> >>> "backend.\n");
> >>>diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >>>index 9cb0e9a..83c962f 100644
> >>>--- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >>>+++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >>>@@ -324,6 +324,10 @@ emit_store(
> >>> }
> >>>
> >>> switch(reg->Register.File) {
> >>>+ case TGSI_FILE_ADDRESS:
> >>>+ temp_ptr = bld->addr[reg->Register.Index][chan_index];
> >>>+ LLVMBuildStore(builder, value, temp_ptr);
> >>>+ continue;
> >>> case TGSI_FILE_OUTPUT:
> >>> temp_ptr = bld->outputs[reg->Register.Index][chan_index];
> >>> break;
> >>>--
> >>>1.8.0.1
> >>>
> >>>_______________________________________________
> >>>mesa-dev mailing list
> >>>mesa-dev at lists.freedesktop.org
> >>>http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >_______________________________________________
> >mesa-dev mailing list
> >mesa-dev at lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list