[Mesa-dev] [PATCH 03/18] ac: add load_tes_inputs() to the abi
Timothy Arceri
tarceri at itsqueeze.com
Tue Dec 12 10:33:34 UTC 2017
On 12/12/17 21:21, Timothy Arceri wrote:
> 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.c
>>> index 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.
Actually this might have been the same issue I described in my reply to
the cover i.e. using get_def_type(ctx, &instr->dest.ssa) causing strange
behavior, I guess I could try again to debug the issue.
>
>>
>> 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.h
>>> index 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.c
>>> index 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list