[Mesa-stable] [Mesa-dev] [PATCH v2] ac/nir: set lod to 0 for ac_image_load_mip

[Samuel Pitoiset]

On 04/27/2018 10:50 AM, Nicolai Hähnle wrote:
> On 27.04.2018 10:05, Samuel Pitoiset wrote:
>> On 04/26/2018 01:51 PM, Nicolai Hähnle wrote:
>>> Is there any reason why we can't just use ac_image_load in such cases?
>>
>> For nit_texop_txf_ms we need one more coordinate for the sample index, 
>> if we use ac_image_load we only loads 2 coordinates instead of 4 with 
>> ac_image_load_mip. Though, I have to admit that I'm a bit confused.
> 
> That's odd. But with multisample textures, ac_image_load_mip makes even 
> less sense, considering that those can't actually have mip levels.
> 
> ac_image_load with ac_image_2dmsaa should load 3 coordinates (see 
> ac_num_coords). We should be choosing ac_image_2dmsaa in that case, at 
> least I'd expect the glsl_sampler_dim to be GLSL_SAMPLER_DIM_MS.

The issue was in RADV actually, we used a 2D dimension (instead of 2D 
MSAA) with nir_texop_txf_ms which is wrong. Sorry for the noise and 
thanks for your help.

Please ignore this patch.

> 
> Cheers,
> Nicolai
> 
>>
>> Any thoughts on this?
>>
>>>
>>> On 26.04.2018 13:18, Samuel Pitoiset wrote:
>>>> Otherwise we hit an assertion in ac_build_image_opcode()
>>>> for ac_image_load_mip.
>>>>
>>>> v2: - set only for ac_image_load_mip
>>>>
>>>> Fixes: 24fb3e6aa16 ("ac/nir: use ac_build_image_opcode for image 
>>>> intrinsics")
>>>> Cc: 18.1 <mesa-stable at lists.freedesktop.org>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/amd/common/ac_nir_to_llvm.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/src/amd/common/ac_nir_to_llvm.c 
>>>> b/src/amd/common/ac_nir_to_llvm.c
>>>> index e4ae6ef49ad..fb8e44509e9 100644
>>>> --- a/src/amd/common/ac_nir_to_llvm.c
>>>> +++ b/src/amd/common/ac_nir_to_llvm.c
>>>> @@ -1276,6 +1276,8 @@ static LLVMValueRef build_tex_intrinsic(struct 
>>>> ac_nir_context *ctx,
>>>>           args->opcode = args->level_zero ||
>>>>                      instr->sampler_dim == GLSL_SAMPLER_DIM_MS ?
>>>>                       ac_image_load : ac_image_load_mip;
>>>> +        if (args->opcode == ac_image_load_mip && !args->lod)
>>>> +            args->lod = ctx->ac.i32_0;
>>>>           args->level_zero = false;
>>>>           break;
>>>>       case nir_texop_txs:
>>>>
>>>
>>>
> 
>