Re: [Mesa-dev] [Mesa-stable] [PATCH] ac: fix the number of coordinates for ac_image_get_lod and arrays

[Nicolai Hähnle]

On 23.04.2018 21:45, Samuel Pitoiset wrote:



On 04/23/2018 08:42 PM, Marek Olšák wrote:
On Mon, Apr 23, 2018 at 1:12 PM, Samuel Pitoiset 
> wrote:




    On 04/23/2018 06:55 PM, Nicolai Hähnle wrote:

    On 23.04.2018 17:52, Samuel Pitoiset wrote:

    This fixes crashes for the following CTS:
    dEQP-VK.glsl.texture_functions.query.texturequerylod.*

    Fixes: 625dcbbc456 ("amd/common: pass address components
    individually to
    ac_build_image_intrinsic")
    Cc: 18.1 >
    Signed-off-by: Samuel Pitoiset >
    ---
   src/amd/common/ac_llvm_build.c | 13 +
   1 file changed, 13 insertions(+)

    diff --git a/src/amd/common/ac_llvm_build.c
    b/src/amd/common/ac_llvm_build.c
    index 02739f9da9c..d5bad3eeea3 100644
    --- a/src/amd/common/ac_llvm_build.c
    +++ b/src/amd/common/ac_llvm_build.c
    @@ -1521,6 +1521,19 @@ LLVMValueRef
    ac_build_image_opcode(struct ac_llvm_context *ctx,
   LLVMValueRef addr;
   unsigned num_addr = 0;
    +    if (a->opcode == ac_image_get_lod) {
    +    switch (a->dim) {
    +    case ac_image_1darray:
    +    num_coords = 1;
    +    break;
    +    case ac_image_2darray:
    +    num_coords = 2;


    This is probably also needed for cube maps, isn't it?


    cubes should have 3 coordinates for textureQueryLod(), no?


Cubemaps are the same as 2D arrays.


Okay. CTS doesn't seem to have tests for cubemaps but that makes sense 
anyway. I'm going to push the patch with the cubemaps fix.


You know, now I'm not so sure anymore :}

The coordinates sent for cubemaps are s/t/layer like for 2D arrays, like 
Marek said and I was thinking yesterday, but there's special handling 
for cubemap corners. We really need a test for that though, because the 
docs aren't too clear either.


Cheers,
Nicolai





Marek




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH] ac: fix the number of coordinates for ac_image_get_lod and arrays

[Samuel Pitoiset]

On 04/23/2018 08:42 PM, Marek Olšák wrote:
On Mon, Apr 23, 2018 at 1:12 PM, Samuel Pitoiset 
> wrote:




On 04/23/2018 06:55 PM, Nicolai Hähnle wrote:

On 23.04.2018 17:52, Samuel Pitoiset wrote:

This fixes crashes for the following CTS:
dEQP-VK.glsl.texture_functions.query.texturequerylod.*

Fixes: 625dcbbc456 ("amd/common: pass address components
individually to
ac_build_image_intrinsic")
Cc: 18.1 >
Signed-off-by: Samuel Pitoiset >
---
   src/amd/common/ac_llvm_build.c | 13 +
   1 file changed, 13 insertions(+)

diff --git a/src/amd/common/ac_llvm_build.c
b/src/amd/common/ac_llvm_build.c
index 02739f9da9c..d5bad3eeea3 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -1521,6 +1521,19 @@ LLVMValueRef
ac_build_image_opcode(struct ac_llvm_context *ctx,
   LLVMValueRef addr;
   unsigned num_addr = 0;
+    if (a->opcode == ac_image_get_lod) {
+    switch (a->dim) {
+    case ac_image_1darray:
+    num_coords = 1;
+    break;
+    case ac_image_2darray:
+    num_coords = 2;


This is probably also needed for cube maps, isn't it?


cubes should have 3 coordinates for textureQueryLod(), no?


Cubemaps are the same as 2D arrays.


Okay. CTS doesn't seem to have tests for cubemaps but that makes sense 
anyway. I'm going to push the patch with the cubemaps fix.




Marek


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH] ac: fix the number of coordinates for ac_image_get_lod and arrays

[Marek Olšák]

On Mon, Apr 23, 2018 at 1:12 PM, Samuel Pitoiset 
wrote:

>
>
> On 04/23/2018 06:55 PM, Nicolai Hähnle wrote:
>
>> On 23.04.2018 17:52, Samuel Pitoiset wrote:
>>
>>> This fixes crashes for the following CTS:
>>> dEQP-VK.glsl.texture_functions.query.texturequerylod.*
>>>
>>> Fixes: 625dcbbc456 ("amd/common: pass address components individually to
>>> ac_build_image_intrinsic")
>>> Cc: 18.1 
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>   src/amd/common/ac_llvm_build.c | 13 +
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/src/amd/common/ac_llvm_build.c
>>> b/src/amd/common/ac_llvm_build.c
>>> index 02739f9da9c..d5bad3eeea3 100644
>>> --- a/src/amd/common/ac_llvm_build.c
>>> +++ b/src/amd/common/ac_llvm_build.c
>>> @@ -1521,6 +1521,19 @@ LLVMValueRef ac_build_image_opcode(struct
>>> ac_llvm_context *ctx,
>>>   LLVMValueRef addr;
>>>   unsigned num_addr = 0;
>>> +if (a->opcode == ac_image_get_lod) {
>>> +switch (a->dim) {
>>> +case ac_image_1darray:
>>> +num_coords = 1;
>>> +break;
>>> +case ac_image_2darray:
>>> +num_coords = 2;
>>>
>>
>> This is probably also needed for cube maps, isn't it?
>>
>
> cubes should have 3 coordinates for textureQueryLod(), no?
>

Cubemaps are the same as 2D arrays.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev