From 52f850238a716af92b4d3eb6c22c6e5075708abb Mon Sep 17 00:00:00 2001 From: Rhys Perry Date: Wed, 4 Aug 2021 14:06:47 +0100 Subject: [PATCH] radv,aco: lower buffer descriptor loads in NIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fossil-db (Sienna Cichlid): Totals from 75420 (46.47% of 162293) affected shaders: MaxWaves: 1878200 -> 1879228 (+0.05%); split: +0.06%, -0.00% Instrs: 54021103 -> 54141370 (+0.22%); split: -0.04%, +0.26% CodeSize: 287813520 -> 288293352 (+0.17%); split: -0.04%, +0.21% VGPRs: 3267576 -> 3266296 (-0.04%); split: -0.04%, +0.00% SpillSGPRs: 10445 -> 10904 (+4.39%); split: -0.31%, +4.70% SpillVGPRs: 1818 -> 1811 (-0.39%); split: -1.05%, +0.66% Scratch: 955392 -> 954368 (-0.11%) Latency: 563477854 -> 562131282 (-0.24%); split: -0.31%, +0.08% InvThroughput: 111860104 -> 111553968 (-0.27%); split: -0.30%, +0.02% VClause: 958432 -> 961415 (+0.31%); split: -0.34%, +0.65% SClause: 1917415 -> 1926952 (+0.50%); split: -0.69%, +1.19% Copies: 3812945 -> 3916758 (+2.72%); split: -0.27%, +2.99% Branches: 1611235 -> 1612022 (+0.05%); split: -0.04%, +0.08% PreSGPRs: 3095505 -> 3126580 (+1.00%); split: -0.06%, +1.07% PreVGPRs: 2773011 -> 2773013 (+0.00%) Most regressions seem to be because ACO's convert_pointer_to_64_bit() can't be CSE'd with radv_nir_apply_pipeline_layout()'s convert_pointer_to_64_bit(). This should be improved by later commits. Signed-off-by: Rhys Perry Reviewed-by: Samuel Pitoiset Reviewed-by: Timur Kristóf Part-of: --- .../compiler/aco_instruction_selection.cpp | 49 +--------- .../aco_instruction_selection_setup.cpp | 1 - src/amd/llvm/ac_nir_to_llvm.c | 8 +- src/amd/llvm/ac_shader_abi.h | 4 +- .../vulkan/radv_nir_apply_pipeline_layout.c | 55 ++++++++++- src/amd/vulkan/radv_nir_to_llvm.c | 95 ++++--------------- .../radeonsi/si_shader_llvm_resources.c | 4 +- 7 files changed, 82 insertions(+), 134 deletions(-) diff --git a/src/amd/compiler/aco_instruction_selection.cpp b/src/amd/compiler/aco_instruction_selection.cpp index c499100fc8c..504b54253fe 100644 --- a/src/amd/compiler/aco_instruction_selection.cpp +++ b/src/amd/compiler/aco_instruction_selection.cpp @@ -5431,28 +5431,12 @@ load_buffer(isel_context* ctx, unsigned num_components, unsigned component_size, emit_load(ctx, bld, info, mubuf_load_params); } -Temp -load_buffer_rsrc(isel_context* ctx, Temp rsrc) -{ - Builder bld(ctx->program, ctx->block); - Temp set_ptr = emit_extract_vector(ctx, rsrc, 0, RegClass(rsrc.type(), 1)); - Temp binding = bld.as_uniform(emit_extract_vector(ctx, rsrc, 1, RegClass(rsrc.type(), 1))); - set_ptr = convert_pointer_to_64_bit(ctx, set_ptr); - return bld.smem(aco_opcode::s_load_dwordx4, bld.def(s4), set_ptr, binding); -} - void visit_load_ubo(isel_context* ctx, nir_intrinsic_instr* instr) { Temp dst = get_ssa_temp(ctx, &instr->dest.ssa); - Temp rsrc = get_ssa_temp(ctx, instr->src[0].ssa); - Builder bld(ctx->program, ctx->block); - - if (rsrc.bytes() == 16) - rsrc = bld.as_uniform(rsrc); /* for VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT */ - else - rsrc = load_buffer_rsrc(ctx, rsrc); + Temp rsrc = bld.as_uniform(get_ssa_temp(ctx, instr->src[0].ssa)); unsigned size = instr->dest.ssa.bit_size / 8; load_buffer(ctx, instr->num_components, size, dst, rsrc, get_ssa_temp(ctx, instr->src[1].ssa), @@ -6403,7 +6387,7 @@ visit_load_ssbo(isel_context* ctx, nir_intrinsic_instr* instr) unsigned num_components = instr->num_components; Temp dst = get_ssa_temp(ctx, &instr->dest.ssa); - Temp rsrc = load_buffer_rsrc(ctx, get_ssa_temp(ctx, instr->src[0].ssa)); + Temp rsrc = bld.as_uniform(get_ssa_temp(ctx, instr->src[0].ssa)); unsigned access = nir_intrinsic_access(instr); bool glc = access & (ACCESS_VOLATILE | ACCESS_COHERENT); @@ -6425,7 +6409,7 @@ visit_store_ssbo(isel_context* ctx, nir_intrinsic_instr* instr) unsigned writemask = util_widen_mask(nir_intrinsic_write_mask(instr), elem_size_bytes); Temp offset = get_ssa_temp(ctx, instr->src[2].ssa); - Temp rsrc = load_buffer_rsrc(ctx, get_ssa_temp(ctx, instr->src[1].ssa)); + Temp rsrc = bld.as_uniform(get_ssa_temp(ctx, instr->src[1].ssa)); memory_sync_info sync = get_memory_sync_info(instr, storage_buffer, 0); bool glc = @@ -6475,7 +6459,7 @@ visit_atomic_ssbo(isel_context* ctx, nir_intrinsic_instr* instr) get_ssa_temp(ctx, instr->src[3].ssa), data); Temp offset = get_ssa_temp(ctx, instr->src[1].ssa); - Temp rsrc = load_buffer_rsrc(ctx, get_ssa_temp(ctx, instr->src[0].ssa)); + Temp rsrc = bld.as_uniform(get_ssa_temp(ctx, instr->src[0].ssa)); Temp dst = get_ssa_temp(ctx, &instr->dest.ssa); @@ -6552,30 +6536,6 @@ visit_atomic_ssbo(isel_context* ctx, nir_intrinsic_instr* instr) ctx->block->instructions.emplace_back(std::move(mubuf)); } -void -visit_get_ssbo_size(isel_context* ctx, nir_intrinsic_instr* instr) -{ - - Temp rsrc = get_ssa_temp(ctx, instr->src[0].ssa); - Temp dst = get_ssa_temp(ctx, &instr->dest.ssa); - bool non_uniform = dst.type() == RegType::vgpr; - - Builder bld(ctx->program, ctx->block); - if (non_uniform) { - Temp set_ptr = emit_extract_vector(ctx, rsrc, 0, RegClass(rsrc.type(), 1)); - Temp binding = emit_extract_vector(ctx, rsrc, 1, RegClass(rsrc.type(), 1)); - Temp index = bld.vadd32(bld.def(v1), set_ptr, binding); - index = convert_pointer_to_64_bit(ctx, index, non_uniform); - - LoadEmitInfo info = {Operand(index), dst, 1, 4}; - info.align_mul = 4; - info.const_offset = 8; - emit_load(ctx, bld, info, global_load_params); - } else { - emit_extract_vector(ctx, load_buffer_rsrc(ctx, rsrc), 2, dst); - } -} - void visit_load_global(isel_context* ctx, nir_intrinsic_instr* instr) { @@ -8095,7 +8055,6 @@ visit_intrinsic(isel_context* ctx, nir_intrinsic_instr* instr) case nir_intrinsic_ssbo_atomic_fmax: visit_atomic_ssbo(ctx, instr); break; case nir_intrinsic_load_scratch: visit_load_scratch(ctx, instr); break; case nir_intrinsic_store_scratch: visit_store_scratch(ctx, instr); break; - case nir_intrinsic_get_ssbo_size: visit_get_ssbo_size(ctx, instr); break; case nir_intrinsic_scoped_barrier: emit_scoped_barrier(ctx, instr); break; case nir_intrinsic_load_num_workgroups: { Temp dst = get_ssa_temp(ctx, &instr->dest.ssa); diff --git a/src/amd/compiler/aco_instruction_selection_setup.cpp b/src/amd/compiler/aco_instruction_selection_setup.cpp index da5412696e2..b359dceb7a9 100644 --- a/src/amd/compiler/aco_instruction_selection_setup.cpp +++ b/src/amd/compiler/aco_instruction_selection_setup.cpp @@ -749,7 +749,6 @@ init_context(isel_context* ctx, nir_shader* shader) case nir_intrinsic_load_ssbo: case nir_intrinsic_load_global: case nir_intrinsic_load_global_constant: - case nir_intrinsic_get_ssbo_size: type = nir_dest_is_divergent(intrinsic->dest) ? RegType::vgpr : RegType::sgpr; break; case nir_intrinsic_load_view_index: diff --git a/src/amd/llvm/ac_nir_to_llvm.c b/src/amd/llvm/ac_nir_to_llvm.c index 4e92396ebf3..7b1e593d663 100644 --- a/src/amd/llvm/ac_nir_to_llvm.c +++ b/src/amd/llvm/ac_nir_to_llvm.c @@ -2259,10 +2259,8 @@ static LLVMValueRef visit_load_ubo_buffer(struct ac_nir_context *ctx, nir_intrin LLVMValueRef offset = get_src(ctx, instr->src[1]); int num_components = instr->num_components; - if (ctx->abi->load_ubo) { - nir_binding binding = nir_chase_binding(instr->src[0]); - rsrc = ctx->abi->load_ubo(ctx->abi, binding.desc_set, binding.binding, binding.success, rsrc); - } + if (ctx->abi->load_ubo) + rsrc = ctx->abi->load_ubo(ctx->abi, rsrc); /* Convert to a scalar 32-bit load. */ if (instr->dest.ssa.bit_size == 64) @@ -4352,7 +4350,7 @@ static LLVMValueRef get_bindless_index_from_uniform(struct ac_nir_context *ctx, index = LLVMBuildMul(ctx->ac.builder, index, LLVMConstInt(ctx->ac.i32, 8, 0), ""); offset = LLVMBuildAdd(ctx->ac.builder, offset, index, ""); - LLVMValueRef ubo_index = ctx->abi->load_ubo(ctx->abi, 0, 0, false, ctx->ac.i32_0); + LLVMValueRef ubo_index = ctx->abi->load_ubo(ctx->abi, ctx->ac.i32_0); LLVMValueRef ret = ac_build_buffer_load(&ctx->ac, ubo_index, 1, NULL, offset, NULL, 0, ctx->ac.f32, 0, true, true); diff --git a/src/amd/llvm/ac_shader_abi.h b/src/amd/llvm/ac_shader_abi.h index 205dfc04df5..0e7126831e4 100644 --- a/src/amd/llvm/ac_shader_abi.h +++ b/src/amd/llvm/ac_shader_abi.h @@ -99,9 +99,7 @@ struct ac_shader_abi { LLVMValueRef (*load_tess_level)(struct ac_shader_abi *abi, unsigned varying_id, bool load_default_state); - LLVMValueRef (*load_ubo)(struct ac_shader_abi *abi, - unsigned desc_set, unsigned binding, - bool valid_binding, LLVMValueRef index); + LLVMValueRef (*load_ubo)(struct ac_shader_abi *abi, LLVMValueRef index); /** * Load the descriptor for the given buffer. diff --git a/src/amd/vulkan/radv_nir_apply_pipeline_layout.c b/src/amd/vulkan/radv_nir_apply_pipeline_layout.c index ba8263c7fb4..3901972be64 100644 --- a/src/amd/vulkan/radv_nir_apply_pipeline_layout.c +++ b/src/amd/vulkan/radv_nir_apply_pipeline_layout.c @@ -170,7 +170,8 @@ load_inline_buffer_descriptor(nir_builder *b, apply_layout_state *state, nir_ssa } static nir_ssa_def * -load_buffer_descriptor(nir_builder *b, apply_layout_state *state, nir_ssa_def *rsrc) +load_buffer_descriptor(nir_builder *b, apply_layout_state *state, nir_ssa_def *rsrc, + unsigned access) { nir_binding binding = nir_chase_binding(nir_src_for_ssa(rsrc)); @@ -186,7 +187,35 @@ load_buffer_descriptor(nir_builder *b, apply_layout_state *state, nir_ssa_def *r } } - return rsrc; + if (access & ACCESS_NON_UNIFORM) + return nir_iadd(b, nir_channel(b, rsrc, 0), nir_channel(b, rsrc, 1)); + + nir_ssa_def *desc_set = convert_pointer_to_64_bit(b, state, nir_channel(b, rsrc, 0)); + return nir_load_smem_amd(b, 4, desc_set, nir_channel(b, rsrc, 1), .align_mul = 16); +} + +static void +visit_get_ssbo_size(nir_builder *b, apply_layout_state *state, nir_intrinsic_instr *intrin) +{ + nir_ssa_def *rsrc = intrin->src[0].ssa; + + nir_ssa_def *size; + if (nir_intrinsic_access(intrin) & ACCESS_NON_UNIFORM) { + nir_ssa_def *ptr = nir_iadd(b, nir_channel(b, rsrc, 0), nir_channel(b, rsrc, 1)); + ptr = nir_iadd_imm(b, ptr, 8); + ptr = convert_pointer_to_64_bit(b, state, ptr); + size = + nir_build_load_global(b, 4, 32, ptr, .access = ACCESS_NON_WRITEABLE | ACCESS_CAN_REORDER, + .align_mul = 16, .align_offset = 4); + } else { + /* load the entire descriptor so it can be CSE'd */ + nir_ssa_def *ptr = convert_pointer_to_64_bit(b, state, nir_channel(b, rsrc, 0)); + nir_ssa_def *desc = nir_load_smem_amd(b, 4, ptr, nir_channel(b, rsrc, 1), .align_mul = 16); + size = nir_channel(b, desc, 2); + } + + nir_ssa_def_rewrite_uses(&intrin->dest.ssa, size); + nir_instr_remove(&intrin->instr); } static void @@ -206,9 +235,29 @@ apply_layout_to_intrin(nir_builder *b, apply_layout_state *state, nir_intrinsic_ visit_load_vulkan_descriptor(b, state, intrin); break; case nir_intrinsic_load_ubo: - rsrc = load_buffer_descriptor(b, state, intrin->src[0].ssa); + case nir_intrinsic_load_ssbo: + case nir_intrinsic_ssbo_atomic_add: + case nir_intrinsic_ssbo_atomic_imin: + case nir_intrinsic_ssbo_atomic_umin: + case nir_intrinsic_ssbo_atomic_fmin: + case nir_intrinsic_ssbo_atomic_imax: + case nir_intrinsic_ssbo_atomic_umax: + case nir_intrinsic_ssbo_atomic_fmax: + case nir_intrinsic_ssbo_atomic_and: + case nir_intrinsic_ssbo_atomic_or: + case nir_intrinsic_ssbo_atomic_xor: + case nir_intrinsic_ssbo_atomic_exchange: + case nir_intrinsic_ssbo_atomic_comp_swap: + rsrc = load_buffer_descriptor(b, state, intrin->src[0].ssa, nir_intrinsic_access(intrin)); nir_instr_rewrite_src_ssa(&intrin->instr, &intrin->src[0], rsrc); break; + case nir_intrinsic_store_ssbo: + rsrc = load_buffer_descriptor(b, state, intrin->src[1].ssa, nir_intrinsic_access(intrin)); + nir_instr_rewrite_src_ssa(&intrin->instr, &intrin->src[1], rsrc); + break; + case nir_intrinsic_get_ssbo_size: + visit_get_ssbo_size(b, state, intrin); + break; default: break; } diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c index 91f1f25ae05..ea93ed5b7fa 100644 --- a/src/amd/vulkan/radv_nir_to_llvm.c +++ b/src/amd/vulkan/radv_nir_to_llvm.c @@ -365,89 +365,36 @@ radv_load_base_vertex(struct ac_shader_abi *abi, bool non_indexed_is_zero) } static LLVMValueRef -get_desc_ptr(struct radv_shader_context *ctx, LLVMValueRef ptr, bool non_uniform) +radv_load_rsrc(struct radv_shader_context *ctx, LLVMValueRef ptr, LLVMTypeRef type) { - LLVMValueRef set_ptr = ac_llvm_extract_elem(&ctx->ac, ptr, 0); - LLVMValueRef offset = ac_llvm_extract_elem(&ctx->ac, ptr, 1); - ptr = LLVMBuildNUWAdd(ctx->ac.builder, set_ptr, offset, ""); + if (ptr && LLVMTypeOf(ptr) == ctx->ac.i32) { + LLVMValueRef result; - unsigned addr_space = AC_ADDR_SPACE_CONST_32BIT; - if (non_uniform) { - /* 32-bit seems to always use SMEM. addrspacecast from 32-bit -> 64-bit is broken. */ - LLVMValueRef dwords[] = {ptr, - LLVMConstInt(ctx->ac.i32, ctx->options->address32_hi, false)}; - ptr = ac_build_gather_values(&ctx->ac, dwords, 2); - ptr = LLVMBuildBitCast(ctx->ac.builder, ptr, ctx->ac.i64, ""); - addr_space = AC_ADDR_SPACE_CONST; + LLVMTypeRef ptr_type = LLVMPointerType(type, AC_ADDR_SPACE_CONST_32BIT); + ptr = LLVMBuildIntToPtr(ctx->ac.builder, ptr, ptr_type, ""); + LLVMSetMetadata(ptr, ctx->ac.uniform_md_kind, ctx->ac.empty_md); + + result = LLVMBuildLoad(ctx->ac.builder, ptr, ""); + LLVMSetMetadata(result, ctx->ac.invariant_load_md_kind, ctx->ac.empty_md); + + return result; } - return LLVMBuildIntToPtr(ctx->ac.builder, ptr, LLVMPointerType(ctx->ac.v4i32, addr_space), ""); + + return ptr; +} + +static LLVMValueRef +radv_load_ubo(struct ac_shader_abi *abi, LLVMValueRef buffer_ptr) +{ + struct radv_shader_context *ctx = radv_shader_context_from_abi(abi); + return radv_load_rsrc(ctx, buffer_ptr, ctx->ac.v4i32); } static LLVMValueRef radv_load_ssbo(struct ac_shader_abi *abi, LLVMValueRef buffer_ptr, bool write, bool non_uniform) { struct radv_shader_context *ctx = radv_shader_context_from_abi(abi); - LLVMValueRef result; - - buffer_ptr = get_desc_ptr(ctx, buffer_ptr, non_uniform); - if (!non_uniform) - LLVMSetMetadata(buffer_ptr, ctx->ac.uniform_md_kind, ctx->ac.empty_md); - - result = LLVMBuildLoad(ctx->ac.builder, buffer_ptr, ""); - LLVMSetMetadata(result, ctx->ac.invariant_load_md_kind, ctx->ac.empty_md); - LLVMSetAlignment(result, 4); - - return result; -} - -static LLVMValueRef -radv_load_ubo(struct ac_shader_abi *abi, unsigned desc_set, unsigned binding, bool valid_binding, - LLVMValueRef buffer_ptr) -{ - struct radv_shader_context *ctx = radv_shader_context_from_abi(abi); - LLVMValueRef result; - - if (valid_binding) { - struct radv_pipeline_layout *pipeline_layout = ctx->options->layout; - struct radv_descriptor_set_layout *layout = pipeline_layout->set[desc_set].layout; - - if (layout->binding[binding].type == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) { - LLVMValueRef set_ptr = ac_llvm_extract_elem(&ctx->ac, buffer_ptr, 0); - LLVMValueRef offset = ac_llvm_extract_elem(&ctx->ac, buffer_ptr, 1); - buffer_ptr = LLVMBuildNUWAdd(ctx->ac.builder, set_ptr, offset, ""); - - uint32_t desc_type = - S_008F0C_DST_SEL_X(V_008F0C_SQ_SEL_X) | S_008F0C_DST_SEL_Y(V_008F0C_SQ_SEL_Y) | - S_008F0C_DST_SEL_Z(V_008F0C_SQ_SEL_Z) | S_008F0C_DST_SEL_W(V_008F0C_SQ_SEL_W); - - if (ctx->ac.chip_class >= GFX10) { - desc_type |= S_008F0C_FORMAT(V_008F0C_GFX10_FORMAT_32_FLOAT) | - S_008F0C_OOB_SELECT(V_008F0C_OOB_SELECT_RAW) | S_008F0C_RESOURCE_LEVEL(1); - } else { - desc_type |= S_008F0C_NUM_FORMAT(V_008F0C_BUF_NUM_FORMAT_FLOAT) | - S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32); - } - - LLVMValueRef desc_components[4] = { - LLVMBuildPtrToInt(ctx->ac.builder, buffer_ptr, ctx->ac.intptr, ""), - LLVMConstInt(ctx->ac.i32, S_008F04_BASE_ADDRESS_HI(ctx->options->address32_hi), - false), - LLVMConstInt(ctx->ac.i32, 0xffffffff, false), - LLVMConstInt(ctx->ac.i32, desc_type, false), - }; - - return ac_build_gather_values(&ctx->ac, desc_components, 4); - } - } - - buffer_ptr = get_desc_ptr(ctx, buffer_ptr, false); - LLVMSetMetadata(buffer_ptr, ctx->ac.uniform_md_kind, ctx->ac.empty_md); - - result = LLVMBuildLoad(ctx->ac.builder, buffer_ptr, ""); - LLVMSetMetadata(result, ctx->ac.invariant_load_md_kind, ctx->ac.empty_md); - LLVMSetAlignment(result, 4); - - return result; + return radv_load_rsrc(ctx, buffer_ptr, ctx->ac.v4i32); } static LLVMValueRef diff --git a/src/gallium/drivers/radeonsi/si_shader_llvm_resources.c b/src/gallium/drivers/radeonsi/si_shader_llvm_resources.c index 3def734a689..0ded858ad91 100644 --- a/src/gallium/drivers/radeonsi/si_shader_llvm_resources.c +++ b/src/gallium/drivers/radeonsi/si_shader_llvm_resources.c @@ -86,9 +86,7 @@ static LLVMValueRef load_const_buffer_desc_fast_path(struct si_shader_context *c return ac_build_gather_values(&ctx->ac, desc_elems, 4); } -static LLVMValueRef load_ubo(struct ac_shader_abi *abi, - unsigned desc_set, unsigned binding, - bool valid_binding, LLVMValueRef index) +static LLVMValueRef load_ubo(struct ac_shader_abi *abi, LLVMValueRef index) { struct si_shader_context *ctx = si_shader_context_from_abi(abi); struct si_shader_selector *sel = ctx->shader->selector;