On Thu, Jan 10, 2019 at 6:59 AM Timothy Arceri <tarc...@itsqueeze.com> 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.
Otherwise,
Reviewed-by: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>
> + } 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
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
