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

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 at 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

"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 at 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

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 at 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

[+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 at 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 at 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
>