ac: fix texture query LOD for 1D textures on GFX9

Submitted by Samuel Pitoiset on April 25, 2018, 9:58 a.m.

Details

Message ID 20180425095838.16532-1-samuel.pitoiset@gmail.com
State New
Headers show
Series "ac: fix texture query LOD for 1D textures on GFX9" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset April 25, 2018, 9:58 a.m.
1D textures are allocated as 2D which means we only need
one coordinate for texture query LOD.

Fixes: 625dcbbc456 ("amd/common: pass address components individually to
ac_build_image_intrinsic")
Cc: 18.1 <mesa-stable@lists.freedesktop.org>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/common/ac_llvm_build.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index f21a5d2623c..be7379f72ef 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -1533,6 +1533,16 @@  LLVMValueRef ac_build_image_opcode(struct ac_llvm_context *ctx,
 		default:
 			break;
 		}
+
+		/* Fixup for GFX9 which allocates 1D textures as 2D, because at
+		 * this point we don't know the orignal sampler dimension.
+		 */
+		if (ctx->chip_class >= GFX9) {
+			if ((a->dim == ac_image_2darray ||
+			     a->dim == ac_image_2d) && !a->coords[1]) {
+				num_coords = 1;
+			}
+		}
 	}
 
 	if (a->offset)

Comments

On 25.04.2018 11:58, Samuel Pitoiset wrote:
> 1D textures are allocated as 2D which means we only need
> one coordinate for texture query LOD.
> 
> Fixes: 625dcbbc456 ("amd/common: pass address components individually to
> ac_build_image_intrinsic")
> Cc: 18.1 <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>   src/amd/common/ac_llvm_build.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
> index f21a5d2623c..be7379f72ef 100644
> --- a/src/amd/common/ac_llvm_build.c
> +++ b/src/amd/common/ac_llvm_build.c
> @@ -1533,6 +1533,16 @@ LLVMValueRef ac_build_image_opcode(struct ac_llvm_context *ctx,
>   		default:
>   			break;
>   		}
> +
> +		/* Fixup for GFX9 which allocates 1D textures as 2D, because at
> +		 * this point we don't know the orignal sampler dimension.
> +		 */
> +		if (ctx->chip_class >= GFX9) {
> +			if ((a->dim == ac_image_2darray ||
> +			     a->dim == ac_image_2d) && !a->coords[1]) {
> +				num_coords = 1;
> +			}
> +		}

Can we do this fixup in ac_nir_to_llvm instead, please? Pretty sure that 
that's needed for correctness anyway: with this change, the second 
coordinate will be basically random, which can probably affect the 
hardware's LOD even when the texture's height is 1.

Thanks,
Nicolai

>   	}
>   
>   	if (a->offset)
>
On 04/25/2018 04:10 PM, Nicolai Hähnle wrote:
> On 25.04.2018 11:58, Samuel Pitoiset wrote:
>> 1D textures are allocated as 2D which means we only need
>> one coordinate for texture query LOD.
>>
>> Fixes: 625dcbbc456 ("amd/common: pass address components individually to
>> ac_build_image_intrinsic")
>> Cc: 18.1 <mesa-stable@lists.freedesktop.org>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/common/ac_llvm_build.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/src/amd/common/ac_llvm_build.c 
>> b/src/amd/common/ac_llvm_build.c
>> index f21a5d2623c..be7379f72ef 100644
>> --- a/src/amd/common/ac_llvm_build.c
>> +++ b/src/amd/common/ac_llvm_build.c
>> @@ -1533,6 +1533,16 @@ LLVMValueRef ac_build_image_opcode(struct 
>> ac_llvm_context *ctx,
>>           default:
>>               break;
>>           }
>> +
>> +        /* Fixup for GFX9 which allocates 1D textures as 2D, because at
>> +         * this point we don't know the orignal sampler dimension.
>> +         */
>> +        if (ctx->chip_class >= GFX9) {
>> +            if ((a->dim == ac_image_2darray ||
>> +                 a->dim == ac_image_2d) && !a->coords[1]) {
>> +                num_coords = 1;
>> +            }
>> +        }
> 
> Can we do this fixup in ac_nir_to_llvm instead, please? Pretty sure that 
> that's needed for correctness anyway: with this change, the second 
> coordinate will be basically random, which can probably affect the 
> hardware's LOD even when the texture's height is 1.

Yes, I can do that, but what should we put in coords[1]? just zero?

> 
> Thanks,
> Nicolai
> 
>>       }
>>       if (a->offset)
>>
> 
>
On 25.04.2018 16:46, Samuel Pitoiset wrote:
> 
> 
> On 04/25/2018 04:10 PM, Nicolai Hähnle wrote:
>> On 25.04.2018 11:58, Samuel Pitoiset wrote:
>>> 1D textures are allocated as 2D which means we only need
>>> one coordinate for texture query LOD.
>>>
>>> Fixes: 625dcbbc456 ("amd/common: pass address components individually to
>>> ac_build_image_intrinsic")
>>> Cc: 18.1 <mesa-stable@lists.freedesktop.org>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>>> ---
>>>   src/amd/common/ac_llvm_build.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/src/amd/common/ac_llvm_build.c 
>>> b/src/amd/common/ac_llvm_build.c
>>> index f21a5d2623c..be7379f72ef 100644
>>> --- a/src/amd/common/ac_llvm_build.c
>>> +++ b/src/amd/common/ac_llvm_build.c
>>> @@ -1533,6 +1533,16 @@ LLVMValueRef ac_build_image_opcode(struct 
>>> ac_llvm_context *ctx,
>>>           default:
>>>               break;
>>>           }
>>> +
>>> +        /* Fixup for GFX9 which allocates 1D textures as 2D, because at
>>> +         * this point we don't know the orignal sampler dimension.
>>> +         */
>>> +        if (ctx->chip_class >= GFX9) {
>>> +            if ((a->dim == ac_image_2darray ||
>>> +                 a->dim == ac_image_2d) && !a->coords[1]) {
>>> +                num_coords = 1;
>>> +            }
>>> +        }
>>
>> Can we do this fixup in ac_nir_to_llvm instead, please? Pretty sure 
>> that that's needed for correctness anyway: with this change, the 
>> second coordinate will be basically random, which can probably affect 
>> the hardware's LOD even when the texture's height is 1.
> 
> Yes, I can do that, but what should we put in coords[1]? just zero?

Yes, that seems best.

Thanks,
Nicolai


> 
>>
>> Thanks,
>> Nicolai
>>
>>>       }
>>>       if (a->offset)
>>>
>>
>>