On 12/12/17 20:42, Nicolai Hähnle wrote:
On 11.12.2017 03:43, Timothy Arceri wrote:---src/amd/common/ac_nir_to_llvm.c | 66 +++++++++++++++++++++-----------src/amd/common/ac_shader_abi.h | 12 ++++++ src/gallium/drivers/radeonsi/si_shader.c | 1 + 3 files changed, 57 insertions(+), 22 deletions(-)diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.cindex 663b27d265a..78ae25ee147 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -2840,53 +2840,51 @@ store_tcs_output(struct nir_to_llvm_context *ctx, } if (writemask == 0xF) {ac_build_buffer_store_dword(&ctx->ac, ctx->hs_ring_tess_offchip, src, 4,buf_addr, ctx->oc_lds, (base * 4), 1, 0, true, false); } } static LLVMValueRef -load_tes_input(struct nir_to_llvm_context *ctx, - const nir_intrinsic_instr *instr) +load_tes_input(struct ac_shader_abi *abi, + LLVMValueRef vertex_index, + LLVMValueRef param_index, + unsigned const_index, + unsigned location, + unsigned driver_location, + LLVMTypeRef type, + unsigned component, + unsigned num_components, + bool is_patch, + bool is_compact) { + struct nir_to_llvm_context *ctx = nir_to_llvm_context_from_abi(abi); LLVMValueRef buf_addr; LLVMValueRef result; - LLVMValueRef vertex_index = NULL; - LLVMValueRef indir_index = NULL; - unsigned const_index = 0; - unsigned param;- const bool per_vertex = nir_is_per_vertex_io(instr->variables[0]->var, ctx->stage);- const bool is_compact = instr->variables[0]->var->data.compact; + unsigned param = shader_io_get_unique_index(location); - get_deref_offset(ctx->nir, instr->variables[0], - false, NULL, per_vertex ? &vertex_index : NULL, - &const_index, &indir_index);- param = shader_io_get_unique_index(instr->variables[0]->var->data.location); - if (instr->variables[0]->var->data.location == VARYING_SLOT_CLIP_DIST0 &&- is_compact && const_index > 3) {+ if (location == VARYING_SLOT_CLIP_DIST0 && is_compact && const_index > 3) {const_index -= 3;So... this is in the original code as well, but why is that -= 3 instead of 4?
Honestly since I didn't have to touch this code I haven't really tried to understand it. Seems like a classic example of why you shouldn't use magic numbers without a comment.
param++; } - unsigned comp = instr->variables[0]->var->data.location_frac;buf_addr = get_tcs_tes_buffer_address_params(ctx, param, const_index,- is_compact, vertex_index, indir_index); + is_compact, vertex_index, param_index);- LLVMValueRef comp_offset = LLVMConstInt(ctx->ac.i32, comp * 4, false); + LLVMValueRef comp_offset = LLVMConstInt(ctx->ac.i32, component * 4, false);buf_addr = LLVMBuildAdd(ctx->builder, buf_addr, comp_offset, "");- result = ac_build_buffer_load(&ctx->ac, ctx->hs_ring_tess_offchip, instr->num_components, NULL, + result = ac_build_buffer_load(&ctx->ac, ctx->hs_ring_tess_offchip, num_components, NULL, buf_addr, ctx->oc_lds, is_compact ? (4 * const_index) : 0, 1, 0, true, false);- result = trim_vector(&ctx->ac, result, instr->num_components);- result = LLVMBuildBitCast(ctx->builder, result, get_def_type(ctx->nir, &instr->dest.ssa), "");+ result = trim_vector(&ctx->ac, result, num_components); return result; } static LLVMValueRef load_gs_input(struct ac_shader_abi *abi, unsigned location, unsigned driver_location, unsigned component, unsigned num_components, unsigned vertex_index,@@ -2988,22 +2986,45 @@ static LLVMValueRef visit_load_var(struct ac_nir_context *ctx,get_deref_offset(ctx, instr->variables[0], vs_in, NULL, NULL, &const_index, &indir_index); if (instr->dest.ssa.bit_size == 64) ve *= 2; switch (instr->variables[0]->var->data.mode) { case nir_var_shader_in: if (ctx->stage == MESA_SHADER_TESS_CTRL) return load_tcs_input(ctx->nctx, instr); - if (ctx->stage == MESA_SHADER_TESS_EVAL) - return load_tes_input(ctx->nctx, instr); + if (ctx->stage == MESA_SHADER_TESS_EVAL) { + LLVMValueRef result; + LLVMValueRef vertex_index = NULL; + LLVMValueRef indir_index = NULL; + unsigned const_index = 0; + unsigned location = instr->variables[0]->var->data.location;+ unsigned driver_location = instr->variables[0]->var->data.driver_location;+ const bool is_patch = instr->variables[0]->var->data.patch;+ const bool is_compact = instr->variables[0]->var->data.compact;+ + get_deref_offset(ctx, instr->variables[0], + false, NULL, is_patch ? NULL : &vertex_index, + &const_index, &indir_index); ++ result = ctx->abi->load_tess_inputs(ctx->abi, vertex_index, indir_index,+ const_index, location, driver_location,+ nir2llvmtype(ctx, instr->variables[0]->var->type), + instr->variables[0]->var->data.location_frac,+ instr->num_components, + is_patch, is_compact); + return ctx->nctx ?+ LLVMBuildBitCast(ctx->nctx->builder, result, get_def_type(ctx, &instr->dest.ssa), "") :+ result;Why is this conditional on ctx->nctx? Could and should probably just use ctx->ac.builder...
I'm fairly certain the bitcast caused problems for the radeonsi path. Maybe this is not the best solution but this is one of a couple of places where the backends need to be massaged in slightly different ways, checking for ctx->nctx is a convenient way to switch paths.
Cheers, Nicolai+ } + if (ctx->stage == MESA_SHADER_GEOMETRY) { LLVMValueRef indir_index; unsigned const_index, vertex_index; get_deref_offset(ctx, instr->variables[0], false, &vertex_index, NULL, &const_index, &indir_index);return ctx->abi->load_inputs(ctx->abi, instr->variables[0]->var->data.location, instr->variables[0]->var->data.driver_location, instr->variables[0]->var->data.location_frac, ve,vertex_index, const_index,@@ -6561,20 +6582,21 @@ LLVMModuleRef ac_translate_nir_to_llvm(LLVMTargetMachineRef tm,if (shaders[i]->info.stage == MESA_SHADER_GEOMETRY) {ctx.gs_next_vertex = ac_build_alloca(&ctx.ac, ctx.ac.i32, "gs_next_vertex");ctx.gs_max_out_vertices = shaders[i]->info.gs.vertices_out; ctx.abi.load_inputs = load_gs_input; } else if (shaders[i]->info.stage == MESA_SHADER_TESS_CTRL) { ctx.tcs_outputs_read = shaders[i]->info.outputs_read;ctx.tcs_patch_outputs_read = shaders[i]->info.patch_outputs_read;} else if (shaders[i]->info.stage == MESA_SHADER_TESS_EVAL) {ctx.tes_primitive_mode = shaders[i]->info.tess.primitive_mode;+ ctx.abi.load_tess_inputs = load_tes_input; } else if (shaders[i]->info.stage == MESA_SHADER_VERTEX) { if (shader_info->info.vs.needs_instance_id) { ctx.shader_info->vs.vgpr_comp_cnt = MAX2(3, ctx.shader_info->vs.vgpr_comp_cnt); } } else if (shaders[i]->info.stage == MESA_SHADER_FRAGMENT) {shader_info->fs.can_discard = shaders[i]->info.fs.uses_discard;} if (i)diff --git a/src/amd/common/ac_shader_abi.h b/src/amd/common/ac_shader_abi.hindex 68fc431d426..f22a6a001e5 100644 --- a/src/amd/common/ac_shader_abi.h +++ b/src/amd/common/ac_shader_abi.h @@ -66,20 +66,32 @@ struct ac_shader_abi { LLVMValueRef (*load_inputs)(struct ac_shader_abi *abi, unsigned location, unsigned driver_location, unsigned component, unsigned num_components, unsigned vertex_index, unsigned const_index, LLVMTypeRef type); + LLVMValueRef (*load_tess_inputs)(struct ac_shader_abi *abi, + LLVMValueRef vertex_index, + LLVMValueRef param_index, + unsigned const_index, + unsigned location, + unsigned driver_location, + LLVMTypeRef type, + unsigned component, + unsigned num_components, + bool is_patch, + bool is_compact); +LLVMValueRef (*load_ubo)(struct ac_shader_abi *abi, LLVMValueRef index);/** * Load the descriptor for the given buffer. ** \param buffer the buffer as presented in NIR: this is the descriptor* in Vulkan, and the buffer index in OpenGL/Gallium * \param write whether buffer contents will be written */ LLVMValueRef (*load_ssbo)(struct ac_shader_abi *abi,diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.cindex df77b28c577..96488ca4e29 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c@@ -5857,20 +5857,21 @@ static bool si_compile_tgsi_main(struct si_shader_context *ctx,bld_base->emit_epilogue = si_tgsi_emit_epilogue; break; case PIPE_SHADER_TESS_CTRL: bld_base->emit_fetch_funcs[TGSI_FILE_INPUT] = fetch_input_tcs;bld_base->emit_fetch_funcs[TGSI_FILE_OUTPUT] = fetch_output_tcs;bld_base->emit_store = store_output_tcs; bld_base->emit_epilogue = si_llvm_emit_tcs_epilogue; break; case PIPE_SHADER_TESS_EVAL: bld_base->emit_fetch_funcs[TGSI_FILE_INPUT] = fetch_input_tes; + ctx->abi.load_tess_inputs = si_nir_load_input_tes; if (shader->key.as_es) ctx->abi.emit_outputs = si_llvm_emit_es_epilogue; else ctx->abi.emit_outputs = si_llvm_emit_vs_epilogue; bld_base->emit_epilogue = si_tgsi_emit_epilogue; break; case PIPE_SHADER_GEOMETRY: bld_base->emit_fetch_funcs[TGSI_FILE_INPUT] = fetch_input_gs; ctx->abi.load_inputs = si_nir_load_input_gs; ctx->abi.emit_vertex = si_llvm_emit_vertex;
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev