[Mesa-dev] [PATCH] ac/nir_to_llvm: fix interpolateAt* for arrays

Timothy Arceri tarceri at itsqueeze.com
Fri Jan 18 23:17:29 UTC 2019



On 19/1/19 9:36 am, Bas Nieuwenhuizen wrote:
> On Thu, Jan 10, 2019 at 6:59 AM Timothy Arceri <tarceri at 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.

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.

> 
> Otherwise,
> 
> Reviewed-by: Bas Nieuwenhuizen <bas at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list