[PATCH mesa v2 1/3] nouveau: codegen: LOAD: Always use component 0 when getting the address

[PATCH mesa v2 1/3] nouveau: codegen: LOAD: Always use component 0 when getting the address

[Hans de Goede]

LOAD loads upto 4 components from the specified resource starting at
the passed in x value of the 2nd source operand, the y, z and w
components of the address should not be used.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-New patch in v2 of this patch-set
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index 557608e..d3c2d61 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -2277,7 +2277,7 @@ Converter::handleLOAD(Value *dst0[4])
          if (!dst0[c])
             continue;
 
-         Value *off = fetchSrc(1, c);
+         Value *off = fetchSrc(1, 0);
          Symbol *sym;
          if (tgsi.getSrc(1).getFile() == TGSI_FILE_IMMEDIATE) {
             off = NULL;
-- 
2.7.3

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH mesa v2 2/3] nouveau: codegen: LOAD: Do not call fetchSrc(1) if the address is immediate

[Hans de Goede]

"off" later gets set to NULL when the address is immediate, so move the
fetchSrc(1) call to the non-immediate branch of the if-else. This brings
handleLOAD's offset handling inline with how it is done in handleSTORE.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-New patch in v2 of this patch-set
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index d3c2d61..e2db731 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -2277,13 +2277,14 @@ Converter::handleLOAD(Value *dst0[4])
          if (!dst0[c])
             continue;
 
-         Value *off = fetchSrc(1, 0);
+         Value *off;
          Symbol *sym;
          if (tgsi.getSrc(1).getFile() == TGSI_FILE_IMMEDIATE) {
             off = NULL;
             sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
                           tgsi.getSrc(1).getValueU32(0, info) + 4 * c);
          } else {
+            off = fetchSrc(1, 0);
             sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c, 4 * c);
          }
 
-- 
2.7.3

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

[PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Hans de Goede]

The llvm TGSI backend uses pointers in registers and does things
like:

LOAD TEMP[0].y, MEMORY[0], TEMP[0]

Expecting the data at address TEMP[0].x to get loaded to
TEMP[0].y. But this will cause the data at TEMP[0].x + 4 to be
loaded instead.

This commit adds support for a swizzle suffix for the 1st source
operand, which allows using:

LOAD TEMP[0].y, MEMORY[0].xxxx, TEMP[0]

And actually getting the desired behavior

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Tweaked commit msg a bit
-Add documentation for this to src/gallium/docs/source/tgsi.rst
---
 src/gallium/docs/source/tgsi.rst                          | 3 +++
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 8 ++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/gallium/docs/source/tgsi.rst b/src/gallium/docs/source/tgsi.rst
index 85c302f..4315707 100644
--- a/src/gallium/docs/source/tgsi.rst
+++ b/src/gallium/docs/source/tgsi.rst
@@ -2288,6 +2288,9 @@ Resource Access Opcodes
                texture arrays and 2D textures.  address.w is always
                ignored.
 
+               A swizzle suffix may be added to the resource argument
+               this will cause the resource data to be swizzled accordingly.
+
 .. opcode:: STORE - Write data to a shader resource
 
                Syntax: ``STORE resource, address, src``
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index e2db731..01df4f3 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -2279,13 +2279,17 @@ Converter::handleLOAD(Value *dst0[4])
 
          Value *off;
          Symbol *sym;
+         uint32_t src0_component_offset = tgsi.getSrc(0).getSwizzle(c) * 4;
+
          if (tgsi.getSrc(1).getFile() == TGSI_FILE_IMMEDIATE) {
             off = NULL;
             sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
-                          tgsi.getSrc(1).getValueU32(0, info) + 4 * c);
+                          tgsi.getSrc(1).getValueU32(0, info) +
+                          src0_component_offset);
          } else {
             off = fetchSrc(1, 0);
-            sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c, 4 * c);
+            sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
+                          src0_component_offset);
          }
 
          Instruction *ld = mkLoad(TYPE_U32, dst0[c], sym, off);
-- 
2.7.3

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Ilia Mirkin]

[+radeon folk]

Marek, Nicolai, Bas - please have a look at the doc change and let us
know if you think this will cause a problem for radeon.

Hans is solving the issue that he wants to swizzle the data loaded
from the image/buffer/whatever before sticking it into the dst
register.

  -ilia

On Thu, Apr 21, 2016 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The llvm TGSI backend uses pointers in registers and does things
> like:
>
> LOAD TEMP[0].y, MEMORY[0], TEMP[0]
>
> Expecting the data at address TEMP[0].x to get loaded to
> TEMP[0].y. But this will cause the data at TEMP[0].x + 4 to be
> loaded instead.
>
> This commit adds support for a swizzle suffix for the 1st source
> operand, which allows using:
>
> LOAD TEMP[0].y, MEMORY[0].xxxx, TEMP[0]
>
> And actually getting the desired behavior
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Tweaked commit msg a bit
> -Add documentation for this to src/gallium/docs/source/tgsi.rst
> ---
>  src/gallium/docs/source/tgsi.rst                          | 3 +++
>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/docs/source/tgsi.rst b/src/gallium/docs/source/tgsi.rst
> index 85c302f..4315707 100644
> --- a/src/gallium/docs/source/tgsi.rst
> +++ b/src/gallium/docs/source/tgsi.rst
> @@ -2288,6 +2288,9 @@ Resource Access Opcodes
>                 texture arrays and 2D textures.  address.w is always
>                 ignored.
>
> +               A swizzle suffix may be added to the resource argument
> +               this will cause the resource data to be swizzled accordingly.
> +
>  .. opcode:: STORE - Write data to a shader resource
>
>                 Syntax: ``STORE resource, address, src``
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index e2db731..01df4f3 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -2279,13 +2279,17 @@ Converter::handleLOAD(Value *dst0[4])
>
>           Value *off;
>           Symbol *sym;
> +         uint32_t src0_component_offset = tgsi.getSrc(0).getSwizzle(c) * 4;
> +
>           if (tgsi.getSrc(1).getFile() == TGSI_FILE_IMMEDIATE) {
>              off = NULL;
>              sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
> -                          tgsi.getSrc(1).getValueU32(0, info) + 4 * c);
> +                          tgsi.getSrc(1).getValueU32(0, info) +
> +                          src0_component_offset);
>           } else {
>              off = fetchSrc(1, 0);
> -            sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c, 4 * c);
> +            sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
> +                          src0_component_offset);
>           }
>
>           Instruction *ld = mkLoad(TYPE_U32, dst0[c], sym, off);
> --
> 2.7.3
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Marek Olšák]

On Thu, Apr 21, 2016 at 7:04 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> [+radeon folk]
>
> Marek, Nicolai, Bas - please have a look at the doc change and let us
> know if you think this will cause a problem for radeon.
>
> Hans is solving the issue that he wants to swizzle the data loaded
> from the image/buffer/whatever before sticking it into the dst
> register.

Is this something st/mesa needs or just nouveau? If just nouveau needs
it, I don't see a point in updating the TGSI spec, since nouveau can
just add the swizzle when translating from TGSI.

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

Re: [PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Hans de Goede]

Hi,

On 22-04-16 09:08, Marek Olšák wrote:
> On Thu, Apr 21, 2016 at 7:04 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> [+radeon folk]
>>
>> Marek, Nicolai, Bas - please have a look at the doc change and let us
>> know if you think this will cause a problem for radeon.
>>
>> Hans is solving the issue that he wants to swizzle the data loaded
>> from the image/buffer/whatever before sticking it into the dst
>> register.
>
> Is this something st/mesa needs or just nouveau? If just nouveau needs
> it, I don't see a point in updating the TGSI spec, since nouveau can
> just add the swizzle when translating from TGSI.

This is something which the llvm tgsi backend needs, which we plan to
use to add opencl support to nouveau.

 From the commit msg:

"The llvm TGSI backend uses pointers in registers and does things like:

LOAD TEMP[0].y, MEMORY[0], TEMP[0]

Expecting the data at address TEMP[0].x to get loaded to
TEMP[0].y. But this will cause the data at TEMP[0].x + 4 to be
loaded instead.

This commit adds support for a swizzle suffix for the 1st source
operand, which allows using:

LOAD TEMP[0].y, MEMORY[0].xxxx, TEMP[0]

And actually getting the desired behavior"

Regards,

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

Re: [PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Marek Olšák]

On Fri, Apr 22, 2016 at 9:23 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 22-04-16 09:08, Marek Olšák wrote:
>>
>> On Thu, Apr 21, 2016 at 7:04 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>
>>> [+radeon folk]
>>>
>>> Marek, Nicolai, Bas - please have a look at the doc change and let us
>>> know if you think this will cause a problem for radeon.
>>>
>>> Hans is solving the issue that he wants to swizzle the data loaded
>>> from the image/buffer/whatever before sticking it into the dst
>>> register.
>>
>>
>> Is this something st/mesa needs or just nouveau? If just nouveau needs
>> it, I don't see a point in updating the TGSI spec, since nouveau can
>> just add the swizzle when translating from TGSI.
>
>
> This is something which the llvm tgsi backend needs, which we plan to
> use to add opencl support to nouveau.
>
> From the commit msg:
>
> "The llvm TGSI backend uses pointers in registers and does things like:
>
> LOAD TEMP[0].y, MEMORY[0], TEMP[0]
>
> Expecting the data at address TEMP[0].x to get loaded to
> TEMP[0].y. But this will cause the data at TEMP[0].x + 4 to be
> loaded instead.
>
> This commit adds support for a swizzle suffix for the 1st source
> operand, which allows using:
>
> LOAD TEMP[0].y, MEMORY[0].xxxx, TEMP[0]
>
> And actually getting the desired behavior"

If radeonsi needs no changes and st/mesa doesn't change behavior, it's OK.

Marek
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa v2 3/3] nouveau: codegen: LOAD: Take src swizzle into account

[Hans de Goede]

Hi,

On 22-04-16 10:37, Marek Olšák wrote:
> On Fri, Apr 22, 2016 at 9:23 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 22-04-16 09:08, Marek Olšák wrote:
>>>
>>> On Thu, Apr 21, 2016 at 7:04 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>
>>>> [+radeon folk]
>>>>
>>>> Marek, Nicolai, Bas - please have a look at the doc change and let us
>>>> know if you think this will cause a problem for radeon.
>>>>
>>>> Hans is solving the issue that he wants to swizzle the data loaded
>>>> from the image/buffer/whatever before sticking it into the dst
>>>> register.
>>>
>>>
>>> Is this something st/mesa needs or just nouveau? If just nouveau needs
>>> it, I don't see a point in updating the TGSI spec, since nouveau can
>>> just add the swizzle when translating from TGSI.
>>
>>
>> This is something which the llvm tgsi backend needs, which we plan to
>> use to add opencl support to nouveau.
>>
>>  From the commit msg:
>>
>> "The llvm TGSI backend uses pointers in registers and does things like:
>>
>> LOAD TEMP[0].y, MEMORY[0], TEMP[0]
>>
>> Expecting the data at address TEMP[0].x to get loaded to
>> TEMP[0].y. But this will cause the data at TEMP[0].x + 4 to be
>> loaded instead.
>>
>> This commit adds support for a swizzle suffix for the 1st source
>> operand, which allows using:
>>
>> LOAD TEMP[0].y, MEMORY[0].xxxx, TEMP[0]
>>
>> And actually getting the desired behavior"
>
> If radeonsi needs no changes and st/mesa doesn't change behavior, it's OK.

Since radeonsi does not use llvm generated tgsi, it should not need any
changes, and these patches do not touch st/mesa.

Regards,

Hans
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau