On 19/1/19 10:29 am, Bas Nieuwenhuizen wrote:
On Sat, Jan 19, 2019 at 12:27 AM Bas Nieuwenhuizen <[email protected]> wrote:On Sat, Jan 19, 2019 at 12:17 AM Timothy Arceri <[email protected]> wrote:On 19/1/19 9:36 am, Bas Nieuwenhuizen wrote:On Thu, Jan 10, 2019 at 6:59 AM Timothy Arceri <[email protected]> wrote:This builds on the recent interpolate fix by Rhys ee8488ea3b99. This doesn't handle arrays of structs but I've got a feeling those might be broken even for radeonsi tgsi (we currently have no tests). This fixes the arb_gpu_shader5 interpolateAt* tests that contain arrays. Fixes: ee8488ea3b99 ("ac/nir,radv,radeonsi/nir: use correct indices for interpolation intrinsics") --- src/amd/common/ac_nir_to_llvm.c | 80 +++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c index 5023b96f92..00011a439d 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -2830,15 +2830,16 @@ static LLVMValueRef visit_interp(struct ac_nir_context *ctx, const nir_intrinsic_instr *instr) { LLVMValueRef result[4]; - LLVMValueRef interp_param, attr_number; + LLVMValueRef interp_param; unsigned location; unsigned chan; LLVMValueRef src_c0 = NULL; LLVMValueRef src_c1 = NULL; LLVMValueRef src0 = NULL; - nir_variable *var = nir_deref_instr_get_variable(nir_instr_as_deref(instr->src[0].ssa->parent_instr)); - int input_index = ctx->abi->fs_input_attr_indices[var->data.location - VARYING_SLOT_VAR0]; + nir_deref_instr *deref_instr = nir_instr_as_deref(instr->src[0].ssa->parent_instr); + nir_variable *var = nir_deref_instr_get_variable(deref_instr); + int input_base = ctx->abi->fs_input_attr_indices[var->data.location - VARYING_SLOT_VAR0]; switch (instr->intrinsic) { case nir_intrinsic_interp_deref_at_centroid: location = INTERP_CENTROID; @@ -2868,7 +2869,6 @@ static LLVMValueRef visit_interp(struct ac_nir_context *ctx, src_c1 = LLVMBuildFSub(ctx->ac.builder, src_c1, halfval, ""); } interp_param = ctx->abi->lookup_interp_param(ctx->abi, var->data.interpolation, location); - attr_number = LLVMConstInt(ctx->ac.i32, input_index, false); if (location == INTERP_CENTER) { LLVMValueRef ij_out[2]; @@ -2906,26 +2906,68 @@ static LLVMValueRef visit_interp(struct ac_nir_context *ctx, } + LLVMValueRef array_idx = ctx->ac.i32_0; + while(deref_instr->deref_type != nir_deref_type_var) { + if (deref_instr->deref_type == nir_deref_type_array) { + unsigned array_size = glsl_get_aoa_size(deref_instr->type); + if (!array_size) + array_size = 1; + + LLVMValueRef offset; + nir_const_value *const_value = nir_src_as_const_value(deref_instr->arr.index); + if (const_value) { + offset = LLVMConstInt(ctx->ac.i32, array_size * const_value->u32[0], false); + } else { + LLVMValueRef indirect = get_src(ctx, deref_instr->arr.index); + + offset = LLVMBuildMul(ctx->ac.builder, indirect, + LLVMConstInt(ctx->ac.i32, array_size, false), ""); + } + + array_idx = LLVMBuildAdd(ctx->ac.builder, array_idx, offset, ""); + deref_instr = nir_src_as_deref(deref_instr->parent); + } else if (deref_instr->deref_type == nir_deref_type_struct) { + /* TODO: Probably need to do more here to support arrays of structs etc */ + deref_instr = nir_src_as_deref(deref_instr->parent);If we don't have confidence this works can we just have it go to the unreachable below. IIRC spirv->nir also lowered struct inputs so I'm not even sure we would encounter this.This will work for structs, just probably not for arrays of structs. We do need struct handling for radeonsi so I'd rather leave this as is.Actually, how does this work for structs? I find it suspicous we don't care about which member is taken?
Yeah your right. It seems the piglit tests are too simple and always use the first member.
I think I will fall through to the unreachable() as you suggested for now. Then I'll write some better tests before adding proper struct support.
Thanks for the review.
Otherwise, Reviewed-by: Bas Nieuwenhuizen <[email protected]>+ } else { + unreachable("Unsupported deref type"); + } + + } + + unsigned input_array_size = glsl_get_aoa_size(var->type); + if (!input_array_size) + input_array_size = 1; + for (chan = 0; chan < 4; chan++) { + LLVMValueRef gather = LLVMGetUndef(LLVMVectorType(ctx->ac.f32, input_array_size)); LLVMValueRef llvm_chan = LLVMConstInt(ctx->ac.i32, chan, false); - if (interp_param) { - interp_param = LLVMBuildBitCast(ctx->ac.builder, + for (unsigned idx = 0; idx < input_array_size; ++idx) { + LLVMValueRef v, attr_number; + + attr_number = LLVMConstInt(ctx->ac.i32, input_base + idx, false); + if (interp_param) { + interp_param = LLVMBuildBitCast(ctx->ac.builder, interp_param, ctx->ac.v2f32, ""); - LLVMValueRef i = LLVMBuildExtractElement( - ctx->ac.builder, interp_param, ctx->ac.i32_0, ""); - LLVMValueRef j = LLVMBuildExtractElement( - ctx->ac.builder, interp_param, ctx->ac.i32_1, ""); - - result[chan] = ac_build_fs_interp(&ctx->ac, - llvm_chan, attr_number, - ctx->abi->prim_mask, i, j); - } else { - result[chan] = ac_build_fs_interp_mov(&ctx->ac, - LLVMConstInt(ctx->ac.i32, 2, false), - llvm_chan, attr_number, - ctx->abi->prim_mask); + LLVMValueRef i = LLVMBuildExtractElement( + ctx->ac.builder, interp_param, ctx->ac.i32_0, ""); + LLVMValueRef j = LLVMBuildExtractElement( + ctx->ac.builder, interp_param, ctx->ac.i32_1, ""); + + v = ac_build_fs_interp(&ctx->ac, llvm_chan, attr_number, + ctx->abi->prim_mask, i, j); + } else { + v = ac_build_fs_interp_mov(&ctx->ac, LLVMConstInt(ctx->ac.i32, 2, false), + llvm_chan, attr_number, ctx->abi->prim_mask); + } + + gather = LLVMBuildInsertElement(ctx->ac.builder, gather, v, + LLVMConstInt(ctx->ac.i32, idx, false), ""); } + + result[chan] = LLVMBuildExtractElement(ctx->ac.builder, gather, array_idx, ""); + } return ac_build_varying_gather_values(&ctx->ac, result, instr->num_components, var->data.location_frac); -- 2.20.1 _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
