[Mesa-dev] [PATCH] ac/nir: fix image dimension for subpass attachments

[Samuel Pitoiset]

On 04/23/2018 01:24 PM, Nicolai Hähnle wrote:
> On 20.04.2018 18:06, Samuel Pitoiset wrote:
>> For subpass attachments we need one more coordinate with
>> the sample index, so make them array types.
> 
> Sorry about that. Shouldn't it be layer index instead of sample index 
> though?

No worries!

Yes, should be layer index. I have pushed the patch on Friday but I'm 
going to send a new one with your suggestion.

> 
> If I understand this right, I think it would be cleaner to just change 
> get_ac_sampler_dim to say:
> 
>      case GLSL_SAMPLER_DIM_SUBPASS:
>          return ac_image_2darray;
>      case GLSL_SAMPLER_DIM_SUBPASS_MS:
>          return ac_image_2darraymsaa;
> 
> That way, the code very clearly documents that subpasses are always of 
> array type.
> 
> Thanks,
> Nicolai
> 
> 
> 
> 
>> This fixes a bunch of CTS fails with RADV.
>>
>> Fixes: 24fb3e6aa1 ("ac/nir: use ac_build_image_opcode for image 
>> intrinsics")
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/common/ac_nir_to_llvm.c | 18 +++++++++++++++---
>>   1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/amd/common/ac_nir_to_llvm.c 
>> b/src/amd/common/ac_nir_to_llvm.c
>> index ba7f353a9a..72c773522f 100644
>> --- a/src/amd/common/ac_nir_to_llvm.c
>> +++ b/src/amd/common/ac_nir_to_llvm.c
>> @@ -2090,6 +2090,18 @@ static LLVMValueRef 
>> adjust_sample_index_using_fmask(struct ac_llvm_context *ctx,
>>       return sample_index;
>>   }
>> +static bool
>> +glsl_is_array_image(const struct glsl_type *type)
>> +{
>> +    const enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
>> +
>> +    if (glsl_sampler_type_is_array(type))
>> +        return true;
>> +
>> +    return dim == GLSL_SAMPLER_DIM_SUBPASS ||
>> +           dim == GLSL_SAMPLER_DIM_SUBPASS_MS;
>> +}
>> +
>>   static void get_image_coords(struct ac_nir_context *ctx,
>>                    const nir_intrinsic_instr *instr,
>>                    struct ac_image_args *args)
>> @@ -2235,7 +2247,7 @@ static LLVMValueRef visit_image_load(struct 
>> ac_nir_context *ctx,
>>           args.resource = get_sampler_desc(ctx, instr->variables[0],
>>                            AC_DESC_IMAGE, NULL, true, false);
>>           args.dim = get_ac_image_dim(&ctx->ac, 
>> glsl_get_sampler_dim(type),
>> -                        glsl_sampler_type_is_array(type));
>> +                        glsl_is_array_image(type));
>>           args.dmask = 15;
>>           args.attributes = AC_FUNC_ATTR_READONLY;
>>           if (var->data.image._volatile || var->data.image.coherent)
>> @@ -2278,7 +2290,7 @@ static void visit_image_store(struct 
>> ac_nir_context *ctx,
>>           args.resource = get_sampler_desc(ctx, instr->variables[0],
>>                            AC_DESC_IMAGE, NULL, true, false);
>>           args.dim = get_ac_image_dim(&ctx->ac, 
>> glsl_get_sampler_dim(type),
>> -                        glsl_sampler_type_is_array(type));
>> +                        glsl_is_array_image(type));
>>           args.dmask = 15;
>>           if (force_glc || var->data.image._volatile || 
>> var->data.image.coherent)
>>               args.cache_policy |= ac_glc;
>> @@ -2369,7 +2381,7 @@ static LLVMValueRef visit_image_atomic(struct 
>> ac_nir_context *ctx,
>>           args.resource = get_sampler_desc(ctx, instr->variables[0],
>>                            AC_DESC_IMAGE, NULL, true, false);
>>           args.dim = get_ac_image_dim(&ctx->ac, 
>> glsl_get_sampler_dim(type),
>> -                        glsl_sampler_type_is_array(type));
>> +                        glsl_is_array_image(type));
>>           return ac_build_image_opcode(&ctx->ac, &args);
>>       }
>>
> 
>