[PATCH mesa 0/3] tgsi and nouveau global / local / opencl-input mem support

[PATCH mesa 0/3] tgsi and nouveau global / local / opencl-input mem support

[Hans de Goede]

Hi,

Here are patches which implement the support for OpenCL kernel input
parameters we discussed. They also add the tgsi parsing bits for
adding support for global / local mem, but no implementation yet.

Regards,

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

[PATCH mesa 1/3] tgsi: Fix decl.Atomic and .Shared not propagating when parsing tgsi text

[Hans de Goede]

When support for decl.Atomic and .Shared was added, tgsi_build_declaration
was not updated to propagate these properly.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/gallium/auxiliary/tgsi/tgsi_build.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c
index cfe9b92..c420ae1 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_build.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
@@ -127,6 +127,8 @@ tgsi_build_declaration(
    unsigned invariant,
    unsigned local,
    unsigned array,
+   unsigned atomic,
+   unsigned shared,
    struct tgsi_header *header )
 {
    struct tgsi_declaration declaration;
@@ -143,6 +145,8 @@ tgsi_build_declaration(
    declaration.Invariant = invariant;
    declaration.Local = local;
    declaration.Array = array;
+   declaration.Atomic = atomic;
+   declaration.Shared = shared;
    header_bodysize_grow( header );
 
    return declaration;
@@ -401,6 +405,8 @@ tgsi_build_full_declaration(
       full_decl->Declaration.Invariant,
       full_decl->Declaration.Local,
       full_decl->Declaration.Array,
+      full_decl->Declaration.Atomic,
+      full_decl->Declaration.Shared,
       header );
 
    if (maxsize <= size)
-- 
2.7.2

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

Re: [PATCH mesa 1/3] tgsi: Fix decl.Atomic and .Shared not propagating when parsing tgsi text

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>

On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> When support for decl.Atomic and .Shared was added, tgsi_build_declaration
> was not updated to propagate these properly.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/gallium/auxiliary/tgsi/tgsi_build.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c
> index cfe9b92..c420ae1 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
> @@ -127,6 +127,8 @@ tgsi_build_declaration(
>     unsigned invariant,
>     unsigned local,
>     unsigned array,
> +   unsigned atomic,
> +   unsigned shared,
>     struct tgsi_header *header )
>  {
>     struct tgsi_declaration declaration;
> @@ -143,6 +145,8 @@ tgsi_build_declaration(
>     declaration.Invariant = invariant;
>     declaration.Local = local;
>     declaration.Array = array;
> +   declaration.Atomic = atomic;
> +   declaration.Shared = shared;
>     header_bodysize_grow( header );
>
>     return declaration;
> @@ -401,6 +405,8 @@ tgsi_build_full_declaration(
>        full_decl->Declaration.Invariant,
>        full_decl->Declaration.Local,
>        full_decl->Declaration.Array,
> +      full_decl->Declaration.Atomic,
> +      full_decl->Declaration.Shared,
>        header );
>
>     if (maxsize <= size)
> --
> 2.7.2
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 1/3] tgsi: Fix decl.Atomic and .Shared not propagating when parsing tgsi text

[Samuel Pitoiset]

Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>


On 03/10/2016 04:14 PM, Hans de Goede wrote:
> When support for decl.Atomic and .Shared was added, tgsi_build_declaration
> was not updated to propagate these properly.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   src/gallium/auxiliary/tgsi/tgsi_build.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c
> index cfe9b92..c420ae1 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
> @@ -127,6 +127,8 @@ tgsi_build_declaration(
>      unsigned invariant,
>      unsigned local,
>      unsigned array,
> +   unsigned atomic,
> +   unsigned shared,
>      struct tgsi_header *header )
>   {
>      struct tgsi_declaration declaration;
> @@ -143,6 +145,8 @@ tgsi_build_declaration(
>      declaration.Invariant = invariant;
>      declaration.Local = local;
>      declaration.Array = array;
> +   declaration.Atomic = atomic;
> +   declaration.Shared = shared;
>      header_bodysize_grow( header );
>
>      return declaration;
> @@ -401,6 +405,8 @@ tgsi_build_full_declaration(
>         full_decl->Declaration.Invariant,
>         full_decl->Declaration.Local,
>         full_decl->Declaration.Array,
> +      full_decl->Declaration.Atomic,
> +      full_decl->Declaration.Shared,
>         header );
>
>      if (maxsize <= size)
>

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

[PATCH mesa 2/3] tgsi: Add support for global / local / input MEMORY

[Hans de Goede]

Extend the MEMORY file support to differentiate between global, local
and shared memory, as well as "input" memory.

"MEMORY[x], INPUT" is intended to access OpenCL kernel parameters, a
special memory type is added for this, since the actual storage of these
(e.g. UBO-s) may differ per implementation. The uploading of kernel
parameters is handled by launch_grid, "MEMORY[x], INPUT" allows drivers
to use an access mechanism for parameter reads which matches with the
upload method.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/gallium/auxiliary/tgsi/tgsi_build.c            |  8 +++----
 src/gallium/auxiliary/tgsi/tgsi_dump.c             |  9 ++++++--
 src/gallium/auxiliary/tgsi/tgsi_text.c             | 14 ++++++++++--
 src/gallium/auxiliary/tgsi/tgsi_ureg.c             | 25 ++++++++++++----------
 src/gallium/auxiliary/tgsi/tgsi_ureg.h             |  2 +-
 .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  |  7 +++---
 src/gallium/include/pipe/p_shader_tokens.h         | 10 +++++++--
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp         |  2 +-
 8 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c
index c420ae1..b108ade 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_build.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
@@ -111,7 +111,7 @@ tgsi_default_declaration( void )
    declaration.Local = 0;
    declaration.Array = 0;
    declaration.Atomic = 0;
-   declaration.Shared = 0;
+   declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
    declaration.Padding = 0;
 
    return declaration;
@@ -128,7 +128,7 @@ tgsi_build_declaration(
    unsigned local,
    unsigned array,
    unsigned atomic,
-   unsigned shared,
+   unsigned mem_type,
    struct tgsi_header *header )
 {
    struct tgsi_declaration declaration;
@@ -146,7 +146,7 @@ tgsi_build_declaration(
    declaration.Local = local;
    declaration.Array = array;
    declaration.Atomic = atomic;
-   declaration.Shared = shared;
+   declaration.MemType = mem_type;
    header_bodysize_grow( header );
 
    return declaration;
@@ -406,7 +406,7 @@ tgsi_build_full_declaration(
       full_decl->Declaration.Local,
       full_decl->Declaration.Array,
       full_decl->Declaration.Atomic,
-      full_decl->Declaration.Shared,
+      full_decl->Declaration.MemType,
       header );
 
    if (maxsize <= size)
diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c b/src/gallium/auxiliary/tgsi/tgsi_dump.c
index f232f38..273f0ae 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
@@ -365,8 +365,13 @@ iter_declaration(
    }
 
    if (decl->Declaration.File == TGSI_FILE_MEMORY) {
-      if (decl->Declaration.Shared)
-         TXT(", SHARED");
+      switch (decl->Declaration.MemType) {
+      /* Note: ,GLOBAL is optional / the default */
+      case TGSI_MEMORY_TYPE_GLOBAL: TXT(", GLOBAL"); break;
+      case TGSI_MEMORY_TYPE_LOCAL:  TXT(", LOCAL");  break;
+      case TGSI_MEMORY_TYPE_SHARED: TXT(", SHARED"); break;
+      case TGSI_MEMORY_TYPE_INPUT:  TXT(", INPUT");  break;
+      }
    }
 
    if (decl->Declaration.File == TGSI_FILE_SAMPLER_VIEW) {
diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c b/src/gallium/auxiliary/tgsi/tgsi_text.c
index 77598d2..9438e3b 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_text.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
@@ -1390,8 +1390,18 @@ static boolean parse_declaration( struct translate_ctx *ctx )
             ctx->cur = cur;
          }
       } else if (file == TGSI_FILE_MEMORY) {
-         if (str_match_nocase_whole(&cur, "SHARED")) {
-            decl.Declaration.Shared = 1;
+         if (str_match_nocase_whole(&cur, "GLOBAL")) {
+            /* Note this is a no-op global is the default */
+            decl.Declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
+            ctx->cur = cur;
+         } else if (str_match_nocase_whole(&cur, "LOCAL")) {
+            decl.Declaration.MemType = TGSI_MEMORY_TYPE_LOCAL;
+            ctx->cur = cur;
+         } else if (str_match_nocase_whole(&cur, "SHARED")) {
+            decl.Declaration.MemType = TGSI_MEMORY_TYPE_SHARED;
+            ctx->cur = cur;
+         } else if (str_match_nocase_whole(&cur, "INPUT")) {
+            decl.Declaration.MemType = TGSI_MEMORY_TYPE_INPUT;
             ctx->cur = cur;
          }
       } else {
diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
index e1a7278..9e10044 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
@@ -190,7 +190,7 @@ struct ureg_program
 
    struct ureg_tokens domain[2];
 
-   bool use_shared_memory;
+   bool use_memory[TGSI_MEMORY_TYPE_COUNT];
 };
 
 static union tgsi_any_token error_tokens[32];
@@ -729,13 +729,14 @@ struct ureg_src ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr,
    return reg;
 }
 
-/* Allocate a shared memory area.
+/* Allocate a memory area.
  */
-struct ureg_src ureg_DECL_shared_memory(struct ureg_program *ureg)
+struct ureg_src ureg_DECL_memory(struct ureg_program *ureg,
+                                 unsigned memory_type)
 {
-   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, 0);
+   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, memory_type);
 
-   ureg->use_shared_memory = true;
+   ureg->use_memory[memory_type] = true;
    return reg;
 }
 
@@ -1666,7 +1667,7 @@ emit_decl_buffer(struct ureg_program *ureg,
 }
 
 static void
-emit_decl_shared_memory(struct ureg_program *ureg)
+emit_decl_memory(struct ureg_program *ureg, unsigned memory_type)
 {
    union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, 2);
 
@@ -1675,11 +1676,11 @@ emit_decl_shared_memory(struct ureg_program *ureg)
    out[0].decl.NrTokens = 2;
    out[0].decl.File = TGSI_FILE_MEMORY;
    out[0].decl.UsageMask = TGSI_WRITEMASK_XYZW;
-   out[0].decl.Shared = true;
+   out[0].decl.MemType = memory_type;
 
    out[1].value = 0;
-   out[1].decl_range.First = 0;
-   out[1].decl_range.Last = 0;
+   out[1].decl_range.First = memory_type;
+   out[1].decl_range.Last = memory_type;
 }
 
 static void
@@ -1854,8 +1855,10 @@ static void emit_decls( struct ureg_program *ureg )
       emit_decl_buffer(ureg, ureg->buffer[i].index, ureg->buffer[i].atomic);
    }
 
-   if (ureg->use_shared_memory)
-      emit_decl_shared_memory(ureg);
+   for (i = 0; i < TGSI_MEMORY_TYPE_COUNT; i++) {
+      if (ureg->use_memory[i])
+         emit_decl_memory(ureg, i);
+   }
 
    if (ureg->const_decls.nr_constant_ranges) {
       for (i = 0; i < ureg->const_decls.nr_constant_ranges; i++) {
diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
index 6a3b5dd..7c7a89e 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
@@ -338,7 +338,7 @@ struct ureg_src
 ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr, bool atomic);
 
 struct ureg_src
-ureg_DECL_shared_memory(struct ureg_program *ureg);
+ureg_DECL_memory(struct ureg_program *ureg, unsigned memory_type);
 
 static inline struct ureg_src
 ureg_imm4f( struct ureg_program *ureg,
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 8683722..a8258af 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -860,7 +860,7 @@ public:
    std::vector<Resource> resources;
 
    struct MemoryFile {
-      bool shared;
+      uint8_t mem_type; // TGSI_MEMORY_TYPE_*
    };
    std::vector<MemoryFile> memoryFiles;
 
@@ -1218,7 +1218,7 @@ bool Source::scanDeclaration(const struct tgsi_full_declaration *decl)
       break;
    case TGSI_FILE_MEMORY:
       for (i = first; i <= last; ++i)
-         memoryFiles[i].shared = decl->Declaration.Shared;
+         memoryFiles[i].mem_type = decl->Declaration.MemType;
       break;
    case TGSI_FILE_NULL:
    case TGSI_FILE_TEMPORARY:
@@ -1523,7 +1523,8 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
 
    sym->reg.fileIndex = fileIdx;
 
-   if (tgsiFile == TGSI_FILE_MEMORY && code->memoryFiles[fileIdx].shared)
+   if (tgsiFile == TGSI_FILE_MEMORY &&
+       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
       sym->setFile(FILE_MEMORY_SHARED);
 
    if (idx >= 0) {
diff --git a/src/gallium/include/pipe/p_shader_tokens.h b/src/gallium/include/pipe/p_shader_tokens.h
index 9d4a96a..0b8d7fb 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -117,6 +117,12 @@ enum tgsi_file_type {
 #define TGSI_CYLINDRICAL_WRAP_Z (1 << 2)
 #define TGSI_CYLINDRICAL_WRAP_W (1 << 3)
 
+#define TGSI_MEMORY_TYPE_GLOBAL        0 /* GLSL global / OpenCL global    */
+#define TGSI_MEMORY_TYPE_LOCAL         1 /* GLSL local  / OpenCL private   */
+#define TGSI_MEMORY_TYPE_SHARED        2 /* GLSL shared / OpenCL local     */
+#define TGSI_MEMORY_TYPE_INPUT         3 /* For OpenCL kernel input params */
+#define TGSI_MEMORY_TYPE_COUNT         4
+
 struct tgsi_declaration
 {
    unsigned Type        : 4;  /**< TGSI_TOKEN_TYPE_DECLARATION */
@@ -130,8 +136,8 @@ struct tgsi_declaration
    unsigned Local       : 1;  /**< optimize as subroutine local variable? */
    unsigned Array       : 1;  /**< extra array info? */
    unsigned Atomic      : 1;  /**< atomic only? for TGSI_FILE_BUFFER */
-   unsigned Shared      : 1;  /**< shared storage for TGSI_FILE_MEMORY */
-   unsigned Padding     : 4;
+   unsigned MemType     : 2;  /**< TGSI_MEMORY_TYPE_x for TGSI_FILE_MEMORY */
+   unsigned Padding     : 3;
 };
 
 struct tgsi_declaration_range
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 26e463e..9b8bf8e 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -6281,7 +6281,7 @@ st_translate_program(
    }
 
    if (program->use_shared_memory)
-      t->shared_memory = ureg_DECL_shared_memory(ureg);
+      t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
 
    for (i = 0; i < program->shader->NumImages; i++) {
       if (program->images_used & (1 << i)) {
-- 
2.7.2

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

Re: [PATCH mesa 2/3] tgsi: Add support for global / local / input MEMORY

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Extend the MEMORY file support to differentiate between global, local
> and shared memory, as well as "input" memory.
>
> "MEMORY[x], INPUT" is intended to access OpenCL kernel parameters, a
> special memory type is added for this, since the actual storage of these
> (e.g. UBO-s) may differ per implementation. The uploading of kernel
> parameters is handled by launch_grid, "MEMORY[x], INPUT" allows drivers
> to use an access mechanism for parameter reads which matches with the
> upload method.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/gallium/auxiliary/tgsi/tgsi_build.c            |  8 +++----
>  src/gallium/auxiliary/tgsi/tgsi_dump.c             |  9 ++++++--
>  src/gallium/auxiliary/tgsi/tgsi_text.c             | 14 ++++++++++--
>  src/gallium/auxiliary/tgsi/tgsi_ureg.c             | 25 ++++++++++++----------
>  src/gallium/auxiliary/tgsi/tgsi_ureg.h             |  2 +-
>  .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  |  7 +++---
>  src/gallium/include/pipe/p_shader_tokens.h         | 10 +++++++--
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp         |  2 +-
>  8 files changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c
> index c420ae1..b108ade 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
> @@ -111,7 +111,7 @@ tgsi_default_declaration( void )
>     declaration.Local = 0;
>     declaration.Array = 0;
>     declaration.Atomic = 0;
> -   declaration.Shared = 0;
> +   declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>     declaration.Padding = 0;
>
>     return declaration;
> @@ -128,7 +128,7 @@ tgsi_build_declaration(
>     unsigned local,
>     unsigned array,
>     unsigned atomic,
> -   unsigned shared,
> +   unsigned mem_type,
>     struct tgsi_header *header )
>  {
>     struct tgsi_declaration declaration;
> @@ -146,7 +146,7 @@ tgsi_build_declaration(
>     declaration.Local = local;
>     declaration.Array = array;
>     declaration.Atomic = atomic;
> -   declaration.Shared = shared;
> +   declaration.MemType = mem_type;
>     header_bodysize_grow( header );
>
>     return declaration;
> @@ -406,7 +406,7 @@ tgsi_build_full_declaration(
>        full_decl->Declaration.Local,
>        full_decl->Declaration.Array,
>        full_decl->Declaration.Atomic,
> -      full_decl->Declaration.Shared,
> +      full_decl->Declaration.MemType,
>        header );
>
>     if (maxsize <= size)
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> index f232f38..273f0ae 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> @@ -365,8 +365,13 @@ iter_declaration(
>     }
>
>     if (decl->Declaration.File == TGSI_FILE_MEMORY) {
> -      if (decl->Declaration.Shared)
> -         TXT(", SHARED");
> +      switch (decl->Declaration.MemType) {
> +      /* Note: ,GLOBAL is optional / the default */
> +      case TGSI_MEMORY_TYPE_GLOBAL: TXT(", GLOBAL"); break;
> +      case TGSI_MEMORY_TYPE_LOCAL:  TXT(", LOCAL");  break;
> +      case TGSI_MEMORY_TYPE_SHARED: TXT(", SHARED"); break;
> +      case TGSI_MEMORY_TYPE_INPUT:  TXT(", INPUT");  break;
> +      }
>     }
>
>     if (decl->Declaration.File == TGSI_FILE_SAMPLER_VIEW) {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c b/src/gallium/auxiliary/tgsi/tgsi_text.c
> index 77598d2..9438e3b 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
> @@ -1390,8 +1390,18 @@ static boolean parse_declaration( struct translate_ctx *ctx )
>              ctx->cur = cur;
>           }
>        } else if (file == TGSI_FILE_MEMORY) {
> -         if (str_match_nocase_whole(&cur, "SHARED")) {
> -            decl.Declaration.Shared = 1;
> +         if (str_match_nocase_whole(&cur, "GLOBAL")) {
> +            /* Note this is a no-op global is the default */
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "LOCAL")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_LOCAL;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "SHARED")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_SHARED;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "INPUT")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_INPUT;
>              ctx->cur = cur;
>           }
>        } else {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> index e1a7278..9e10044 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> @@ -190,7 +190,7 @@ struct ureg_program
>
>     struct ureg_tokens domain[2];
>
> -   bool use_shared_memory;
> +   bool use_memory[TGSI_MEMORY_TYPE_COUNT];
>  };
>
>  static union tgsi_any_token error_tokens[32];
> @@ -729,13 +729,14 @@ struct ureg_src ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr,
>     return reg;
>  }
>
> -/* Allocate a shared memory area.
> +/* Allocate a memory area.
>   */
> -struct ureg_src ureg_DECL_shared_memory(struct ureg_program *ureg)
> +struct ureg_src ureg_DECL_memory(struct ureg_program *ureg,
> +                                 unsigned memory_type)
>  {
> -   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, 0);
> +   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, memory_type);
>
> -   ureg->use_shared_memory = true;
> +   ureg->use_memory[memory_type] = true;
>     return reg;
>  }
>
> @@ -1666,7 +1667,7 @@ emit_decl_buffer(struct ureg_program *ureg,
>  }
>
>  static void
> -emit_decl_shared_memory(struct ureg_program *ureg)
> +emit_decl_memory(struct ureg_program *ureg, unsigned memory_type)
>  {
>     union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, 2);
>
> @@ -1675,11 +1676,11 @@ emit_decl_shared_memory(struct ureg_program *ureg)
>     out[0].decl.NrTokens = 2;
>     out[0].decl.File = TGSI_FILE_MEMORY;
>     out[0].decl.UsageMask = TGSI_WRITEMASK_XYZW;
> -   out[0].decl.Shared = true;
> +   out[0].decl.MemType = memory_type;
>
>     out[1].value = 0;
> -   out[1].decl_range.First = 0;
> -   out[1].decl_range.Last = 0;
> +   out[1].decl_range.First = memory_type;
> +   out[1].decl_range.Last = memory_type;
>  }
>
>  static void
> @@ -1854,8 +1855,10 @@ static void emit_decls( struct ureg_program *ureg )
>        emit_decl_buffer(ureg, ureg->buffer[i].index, ureg->buffer[i].atomic);
>     }
>
> -   if (ureg->use_shared_memory)
> -      emit_decl_shared_memory(ureg);
> +   for (i = 0; i < TGSI_MEMORY_TYPE_COUNT; i++) {
> +      if (ureg->use_memory[i])
> +         emit_decl_memory(ureg, i);
> +   }
>
>     if (ureg->const_decls.nr_constant_ranges) {
>        for (i = 0; i < ureg->const_decls.nr_constant_ranges; i++) {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> index 6a3b5dd..7c7a89e 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> @@ -338,7 +338,7 @@ struct ureg_src
>  ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr, bool atomic);
>
>  struct ureg_src
> -ureg_DECL_shared_memory(struct ureg_program *ureg);
> +ureg_DECL_memory(struct ureg_program *ureg, unsigned memory_type);
>
>  static inline struct ureg_src
>  ureg_imm4f( struct ureg_program *ureg,
> 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 8683722..a8258af 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -860,7 +860,7 @@ public:
>     std::vector<Resource> resources;
>
>     struct MemoryFile {
> -      bool shared;
> +      uint8_t mem_type; // TGSI_MEMORY_TYPE_*
>     };
>     std::vector<MemoryFile> memoryFiles;
>
> @@ -1218,7 +1218,7 @@ bool Source::scanDeclaration(const struct tgsi_full_declaration *decl)
>        break;
>     case TGSI_FILE_MEMORY:
>        for (i = first; i <= last; ++i)
> -         memoryFiles[i].shared = decl->Declaration.Shared;
> +         memoryFiles[i].mem_type = decl->Declaration.MemType;
>        break;
>     case TGSI_FILE_NULL:
>     case TGSI_FILE_TEMPORARY:
> @@ -1523,7 +1523,8 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
>
>     sym->reg.fileIndex = fileIdx;
>
> -   if (tgsiFile == TGSI_FILE_MEMORY && code->memoryFiles[fileIdx].shared)
> +   if (tgsiFile == TGSI_FILE_MEMORY &&
> +       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>        sym->setFile(FILE_MEMORY_SHARED);
>
>     if (idx >= 0) {
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h b/src/gallium/include/pipe/p_shader_tokens.h
> index 9d4a96a..0b8d7fb 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -117,6 +117,12 @@ enum tgsi_file_type {
>  #define TGSI_CYLINDRICAL_WRAP_Z (1 << 2)
>  #define TGSI_CYLINDRICAL_WRAP_W (1 << 3)
>
> +#define TGSI_MEMORY_TYPE_GLOBAL        0 /* GLSL global / OpenCL global    */
> +#define TGSI_MEMORY_TYPE_LOCAL         1 /* GLSL local  / OpenCL private   */
> +#define TGSI_MEMORY_TYPE_SHARED        2 /* GLSL shared / OpenCL local     */
> +#define TGSI_MEMORY_TYPE_INPUT         3 /* For OpenCL kernel input params */

I'm moderately sure that only GLSL shared is a concept in GLSL. The
rest are OpenCL-only things. If you wanted to flip "local" to
"private", I wouldn't object.

With these comments touched up a bit, this patch is

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>

> +#define TGSI_MEMORY_TYPE_COUNT         4
> +
>  struct tgsi_declaration
>  {
>     unsigned Type        : 4;  /**< TGSI_TOKEN_TYPE_DECLARATION */
> @@ -130,8 +136,8 @@ struct tgsi_declaration
>     unsigned Local       : 1;  /**< optimize as subroutine local variable? */
>     unsigned Array       : 1;  /**< extra array info? */
>     unsigned Atomic      : 1;  /**< atomic only? for TGSI_FILE_BUFFER */
> -   unsigned Shared      : 1;  /**< shared storage for TGSI_FILE_MEMORY */
> -   unsigned Padding     : 4;
> +   unsigned MemType     : 2;  /**< TGSI_MEMORY_TYPE_x for TGSI_FILE_MEMORY */
> +   unsigned Padding     : 3;
>  };
>
>  struct tgsi_declaration_range
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 26e463e..9b8bf8e 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -6281,7 +6281,7 @@ st_translate_program(
>     }
>
>     if (program->use_shared_memory)
> -      t->shared_memory = ureg_DECL_shared_memory(ureg);
> +      t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
>
>     for (i = 0; i < program->shader->NumImages; i++) {
>        if (program->images_used & (1 << i)) {
> --
> 2.7.2
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 2/3] tgsi: Add support for global / local / input MEMORY

[Aaron Watry]

[-- Attachment #1.1: Type: text/plain, Size: 11351 bytes --]

On Thu, Mar 10, 2016 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:

> Extend the MEMORY file support to differentiate between global, local
> and shared memory, as well as "input" memory.
>
> "MEMORY[x], INPUT" is intended to access OpenCL kernel parameters, a
> special memory type is added for this, since the actual storage of these
> (e.g. UBO-s) may differ per implementation. The uploading of kernel
> parameters is handled by launch_grid, "MEMORY[x], INPUT" allows drivers
> to use an access mechanism for parameter reads which matches with the
> upload method.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/gallium/auxiliary/tgsi/tgsi_build.c            |  8 +++----
>  src/gallium/auxiliary/tgsi/tgsi_dump.c             |  9 ++++++--
>  src/gallium/auxiliary/tgsi/tgsi_text.c             | 14 ++++++++++--
>  src/gallium/auxiliary/tgsi/tgsi_ureg.c             | 25
> ++++++++++++----------
>  src/gallium/auxiliary/tgsi/tgsi_ureg.h             |  2 +-
>  .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  |  7 +++---
>  src/gallium/include/pipe/p_shader_tokens.h         | 10 +++++++--
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp         |  2 +-
>  8 files changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c
> b/src/gallium/auxiliary/tgsi/tgsi_build.c
> index c420ae1..b108ade 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
> @@ -111,7 +111,7 @@ tgsi_default_declaration( void )
>     declaration.Local = 0;
>     declaration.Array = 0;
>     declaration.Atomic = 0;
> -   declaration.Shared = 0;
> +   declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>     declaration.Padding = 0;
>
>     return declaration;
> @@ -128,7 +128,7 @@ tgsi_build_declaration(
>     unsigned local,
>     unsigned array,
>     unsigned atomic,
> -   unsigned shared,
> +   unsigned mem_type,
>     struct tgsi_header *header )
>  {
>     struct tgsi_declaration declaration;
> @@ -146,7 +146,7 @@ tgsi_build_declaration(
>     declaration.Local = local;
>     declaration.Array = array;
>     declaration.Atomic = atomic;
> -   declaration.Shared = shared;
> +   declaration.MemType = mem_type;
>     header_bodysize_grow( header );
>
>     return declaration;
> @@ -406,7 +406,7 @@ tgsi_build_full_declaration(
>        full_decl->Declaration.Local,
>        full_decl->Declaration.Array,
>        full_decl->Declaration.Atomic,
> -      full_decl->Declaration.Shared,
> +      full_decl->Declaration.MemType,
>        header );
>
>     if (maxsize <= size)
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> index f232f38..273f0ae 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> @@ -365,8 +365,13 @@ iter_declaration(
>     }
>
>     if (decl->Declaration.File == TGSI_FILE_MEMORY) {
> -      if (decl->Declaration.Shared)
> -         TXT(", SHARED");
> +      switch (decl->Declaration.MemType) {
> +      /* Note: ,GLOBAL is optional / the default */
> +      case TGSI_MEMORY_TYPE_GLOBAL: TXT(", GLOBAL"); break;
> +      case TGSI_MEMORY_TYPE_LOCAL:  TXT(", LOCAL");  break;
> +      case TGSI_MEMORY_TYPE_SHARED: TXT(", SHARED"); break;
> +      case TGSI_MEMORY_TYPE_INPUT:  TXT(", INPUT");  break;
> +      }
>     }
>
>     if (decl->Declaration.File == TGSI_FILE_SAMPLER_VIEW) {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
> b/src/gallium/auxiliary/tgsi/tgsi_text.c
> index 77598d2..9438e3b 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
> @@ -1390,8 +1390,18 @@ static boolean parse_declaration( struct
> translate_ctx *ctx )
>              ctx->cur = cur;
>           }
>        } else if (file == TGSI_FILE_MEMORY) {
> -         if (str_match_nocase_whole(&cur, "SHARED")) {
> -            decl.Declaration.Shared = 1;
> +         if (str_match_nocase_whole(&cur, "GLOBAL")) {
> +            /* Note this is a no-op global is the default */
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "LOCAL")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_LOCAL;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "SHARED")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_SHARED;
> +            ctx->cur = cur;
> +         } else if (str_match_nocase_whole(&cur, "INPUT")) {
> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_INPUT;
>              ctx->cur = cur;
>           }
>        } else {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> index e1a7278..9e10044 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> @@ -190,7 +190,7 @@ struct ureg_program
>
>     struct ureg_tokens domain[2];
>
> -   bool use_shared_memory;
> +   bool use_memory[TGSI_MEMORY_TYPE_COUNT];
>  };
>
>  static union tgsi_any_token error_tokens[32];
> @@ -729,13 +729,14 @@ struct ureg_src ureg_DECL_buffer(struct ureg_program
> *ureg, unsigned nr,
>     return reg;
>  }
>
> -/* Allocate a shared memory area.
> +/* Allocate a memory area.
>   */
> -struct ureg_src ureg_DECL_shared_memory(struct ureg_program *ureg)
> +struct ureg_src ureg_DECL_memory(struct ureg_program *ureg,
> +                                 unsigned memory_type)
>  {
> -   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, 0);
> +   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, memory_type);
>
> -   ureg->use_shared_memory = true;
> +   ureg->use_memory[memory_type] = true;
>     return reg;
>  }
>
> @@ -1666,7 +1667,7 @@ emit_decl_buffer(struct ureg_program *ureg,
>  }
>
>  static void
> -emit_decl_shared_memory(struct ureg_program *ureg)
> +emit_decl_memory(struct ureg_program *ureg, unsigned memory_type)
>  {
>     union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, 2);
>
> @@ -1675,11 +1676,11 @@ emit_decl_shared_memory(struct ureg_program *ureg)
>     out[0].decl.NrTokens = 2;
>     out[0].decl.File = TGSI_FILE_MEMORY;
>     out[0].decl.UsageMask = TGSI_WRITEMASK_XYZW;
> -   out[0].decl.Shared = true;
> +   out[0].decl.MemType = memory_type;
>
>     out[1].value = 0;
> -   out[1].decl_range.First = 0;
> -   out[1].decl_range.Last = 0;
> +   out[1].decl_range.First = memory_type;
> +   out[1].decl_range.Last = memory_type;
>  }
>
>  static void
> @@ -1854,8 +1855,10 @@ static void emit_decls( struct ureg_program *ureg )
>        emit_decl_buffer(ureg, ureg->buffer[i].index,
> ureg->buffer[i].atomic);
>     }
>
> -   if (ureg->use_shared_memory)
> -      emit_decl_shared_memory(ureg);
> +   for (i = 0; i < TGSI_MEMORY_TYPE_COUNT; i++) {
> +      if (ureg->use_memory[i])
> +         emit_decl_memory(ureg, i);
> +   }
>
>     if (ureg->const_decls.nr_constant_ranges) {
>        for (i = 0; i < ureg->const_decls.nr_constant_ranges; i++) {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> index 6a3b5dd..7c7a89e 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> @@ -338,7 +338,7 @@ struct ureg_src
>  ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr, bool atomic);
>
>  struct ureg_src
> -ureg_DECL_shared_memory(struct ureg_program *ureg);
> +ureg_DECL_memory(struct ureg_program *ureg, unsigned memory_type);
>
>  static inline struct ureg_src
>  ureg_imm4f( struct ureg_program *ureg,
> 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 8683722..a8258af 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -860,7 +860,7 @@ public:
>     std::vector<Resource> resources;
>
>     struct MemoryFile {
> -      bool shared;
> +      uint8_t mem_type; // TGSI_MEMORY_TYPE_*
>     };
>     std::vector<MemoryFile> memoryFiles;
>
> @@ -1218,7 +1218,7 @@ bool Source::scanDeclaration(const struct
> tgsi_full_declaration *decl)
>        break;
>     case TGSI_FILE_MEMORY:
>        for (i = first; i <= last; ++i)
> -         memoryFiles[i].shared = decl->Declaration.Shared;
> +         memoryFiles[i].mem_type = decl->Declaration.MemType;
>        break;
>     case TGSI_FILE_NULL:
>     case TGSI_FILE_TEMPORARY:
> @@ -1523,7 +1523,8 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
> idx, int c, uint32_t address)
>
>     sym->reg.fileIndex = fileIdx;
>
> -   if (tgsiFile == TGSI_FILE_MEMORY && code->memoryFiles[fileIdx].shared)
> +   if (tgsiFile == TGSI_FILE_MEMORY &&
> +       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>        sym->setFile(FILE_MEMORY_SHARED);
>
>     if (idx >= 0) {
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
> b/src/gallium/include/pipe/p_shader_tokens.h
> index 9d4a96a..0b8d7fb 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -117,6 +117,12 @@ enum tgsi_file_type {
>  #define TGSI_CYLINDRICAL_WRAP_Z (1 << 2)
>  #define TGSI_CYLINDRICAL_WRAP_W (1 << 3)
>
> +#define TGSI_MEMORY_TYPE_GLOBAL        0 /* GLSL global / OpenCL global
>   */
> +#define TGSI_MEMORY_TYPE_LOCAL         1 /* GLSL local  / OpenCL private
>  */
> +#define TGSI_MEMORY_TYPE_SHARED        2 /* GLSL shared / OpenCL local
>  */
> +#define TGSI_MEMORY_TYPE_INPUT         3 /* For OpenCL kernel input
> params */
>

Note: I know little about how you are approaching the CL implementation for
Nouveau...

Should I expect to see an entry in here anywhere for the CONSTANT memory
type?

My GL knowledge is pretty basic. Would that map reasonably to something
like a GL UBO? Do we need an explicit type for CL constant buffers?

--Aaron



> +#define TGSI_MEMORY_TYPE_COUNT         4
> +
>  struct tgsi_declaration
>  {
>     unsigned Type        : 4;  /**< TGSI_TOKEN_TYPE_DECLARATION */
> @@ -130,8 +136,8 @@ struct tgsi_declaration
>     unsigned Local       : 1;  /**< optimize as subroutine local variable?
> */
>     unsigned Array       : 1;  /**< extra array info? */
>     unsigned Atomic      : 1;  /**< atomic only? for TGSI_FILE_BUFFER */
> -   unsigned Shared      : 1;  /**< shared storage for TGSI_FILE_MEMORY */
> -   unsigned Padding     : 4;
> +   unsigned MemType     : 2;  /**< TGSI_MEMORY_TYPE_x for
> TGSI_FILE_MEMORY */
> +   unsigned Padding     : 3;
>  };
>
>  struct tgsi_declaration_range
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 26e463e..9b8bf8e 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -6281,7 +6281,7 @@ st_translate_program(
>     }
>
>     if (program->use_shared_memory)
> -      t->shared_memory = ureg_DECL_shared_memory(ureg);
> +      t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
>
>     for (i = 0; i < program->shader->NumImages; i++) {
>        if (program->images_used & (1 << i)) {
> --
> 2.7.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

[-- Attachment #1.2: Type: text/html, Size: 13827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH mesa 2/3] tgsi: Add support for global / local / input MEMORY

[Hans de Goede]

Hi,

On 10-03-16 16:35, Aaron Watry wrote:
> On Thu, Mar 10, 2016 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Extend the MEMORY file support to differentiate between global, local
>> and shared memory, as well as "input" memory.
>>
>> "MEMORY[x], INPUT" is intended to access OpenCL kernel parameters, a
>> special memory type is added for this, since the actual storage of these
>> (e.g. UBO-s) may differ per implementation. The uploading of kernel
>> parameters is handled by launch_grid, "MEMORY[x], INPUT" allows drivers
>> to use an access mechanism for parameter reads which matches with the
>> upload method.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   src/gallium/auxiliary/tgsi/tgsi_build.c            |  8 +++----
>>   src/gallium/auxiliary/tgsi/tgsi_dump.c             |  9 ++++++--
>>   src/gallium/auxiliary/tgsi/tgsi_text.c             | 14 ++++++++++--
>>   src/gallium/auxiliary/tgsi/tgsi_ureg.c             | 25
>> ++++++++++++----------
>>   src/gallium/auxiliary/tgsi/tgsi_ureg.h             |  2 +-
>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  |  7 +++---
>>   src/gallium/include/pipe/p_shader_tokens.h         | 10 +++++++--
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp         |  2 +-
>>   8 files changed, 51 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c
>> b/src/gallium/auxiliary/tgsi/tgsi_build.c
>> index c420ae1..b108ade 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
>> @@ -111,7 +111,7 @@ tgsi_default_declaration( void )
>>      declaration.Local = 0;
>>      declaration.Array = 0;
>>      declaration.Atomic = 0;
>> -   declaration.Shared = 0;
>> +   declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>>      declaration.Padding = 0;
>>
>>      return declaration;
>> @@ -128,7 +128,7 @@ tgsi_build_declaration(
>>      unsigned local,
>>      unsigned array,
>>      unsigned atomic,
>> -   unsigned shared,
>> +   unsigned mem_type,
>>      struct tgsi_header *header )
>>   {
>>      struct tgsi_declaration declaration;
>> @@ -146,7 +146,7 @@ tgsi_build_declaration(
>>      declaration.Local = local;
>>      declaration.Array = array;
>>      declaration.Atomic = atomic;
>> -   declaration.Shared = shared;
>> +   declaration.MemType = mem_type;
>>      header_bodysize_grow( header );
>>
>>      return declaration;
>> @@ -406,7 +406,7 @@ tgsi_build_full_declaration(
>>         full_decl->Declaration.Local,
>>         full_decl->Declaration.Array,
>>         full_decl->Declaration.Atomic,
>> -      full_decl->Declaration.Shared,
>> +      full_decl->Declaration.MemType,
>>         header );
>>
>>      if (maxsize <= size)
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> index f232f38..273f0ae 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> @@ -365,8 +365,13 @@ iter_declaration(
>>      }
>>
>>      if (decl->Declaration.File == TGSI_FILE_MEMORY) {
>> -      if (decl->Declaration.Shared)
>> -         TXT(", SHARED");
>> +      switch (decl->Declaration.MemType) {
>> +      /* Note: ,GLOBAL is optional / the default */
>> +      case TGSI_MEMORY_TYPE_GLOBAL: TXT(", GLOBAL"); break;
>> +      case TGSI_MEMORY_TYPE_LOCAL:  TXT(", LOCAL");  break;
>> +      case TGSI_MEMORY_TYPE_SHARED: TXT(", SHARED"); break;
>> +      case TGSI_MEMORY_TYPE_INPUT:  TXT(", INPUT");  break;
>> +      }
>>      }
>>
>>      if (decl->Declaration.File == TGSI_FILE_SAMPLER_VIEW) {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> index 77598d2..9438e3b 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> @@ -1390,8 +1390,18 @@ static boolean parse_declaration( struct
>> translate_ctx *ctx )
>>               ctx->cur = cur;
>>            }
>>         } else if (file == TGSI_FILE_MEMORY) {
>> -         if (str_match_nocase_whole(&cur, "SHARED")) {
>> -            decl.Declaration.Shared = 1;
>> +         if (str_match_nocase_whole(&cur, "GLOBAL")) {
>> +            /* Note this is a no-op global is the default */
>> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>> +            ctx->cur = cur;
>> +         } else if (str_match_nocase_whole(&cur, "LOCAL")) {
>> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_LOCAL;
>> +            ctx->cur = cur;
>> +         } else if (str_match_nocase_whole(&cur, "SHARED")) {
>> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_SHARED;
>> +            ctx->cur = cur;
>> +         } else if (str_match_nocase_whole(&cur, "INPUT")) {
>> +            decl.Declaration.MemType = TGSI_MEMORY_TYPE_INPUT;
>>               ctx->cur = cur;
>>            }
>>         } else {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> index e1a7278..9e10044 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> @@ -190,7 +190,7 @@ struct ureg_program
>>
>>      struct ureg_tokens domain[2];
>>
>> -   bool use_shared_memory;
>> +   bool use_memory[TGSI_MEMORY_TYPE_COUNT];
>>   };
>>
>>   static union tgsi_any_token error_tokens[32];
>> @@ -729,13 +729,14 @@ struct ureg_src ureg_DECL_buffer(struct ureg_program
>> *ureg, unsigned nr,
>>      return reg;
>>   }
>>
>> -/* Allocate a shared memory area.
>> +/* Allocate a memory area.
>>    */
>> -struct ureg_src ureg_DECL_shared_memory(struct ureg_program *ureg)
>> +struct ureg_src ureg_DECL_memory(struct ureg_program *ureg,
>> +                                 unsigned memory_type)
>>   {
>> -   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, 0);
>> +   struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, memory_type);
>>
>> -   ureg->use_shared_memory = true;
>> +   ureg->use_memory[memory_type] = true;
>>      return reg;
>>   }
>>
>> @@ -1666,7 +1667,7 @@ emit_decl_buffer(struct ureg_program *ureg,
>>   }
>>
>>   static void
>> -emit_decl_shared_memory(struct ureg_program *ureg)
>> +emit_decl_memory(struct ureg_program *ureg, unsigned memory_type)
>>   {
>>      union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, 2);
>>
>> @@ -1675,11 +1676,11 @@ emit_decl_shared_memory(struct ureg_program *ureg)
>>      out[0].decl.NrTokens = 2;
>>      out[0].decl.File = TGSI_FILE_MEMORY;
>>      out[0].decl.UsageMask = TGSI_WRITEMASK_XYZW;
>> -   out[0].decl.Shared = true;
>> +   out[0].decl.MemType = memory_type;
>>
>>      out[1].value = 0;
>> -   out[1].decl_range.First = 0;
>> -   out[1].decl_range.Last = 0;
>> +   out[1].decl_range.First = memory_type;
>> +   out[1].decl_range.Last = memory_type;
>>   }
>>
>>   static void
>> @@ -1854,8 +1855,10 @@ static void emit_decls( struct ureg_program *ureg )
>>         emit_decl_buffer(ureg, ureg->buffer[i].index,
>> ureg->buffer[i].atomic);
>>      }
>>
>> -   if (ureg->use_shared_memory)
>> -      emit_decl_shared_memory(ureg);
>> +   for (i = 0; i < TGSI_MEMORY_TYPE_COUNT; i++) {
>> +      if (ureg->use_memory[i])
>> +         emit_decl_memory(ureg, i);
>> +   }
>>
>>      if (ureg->const_decls.nr_constant_ranges) {
>>         for (i = 0; i < ureg->const_decls.nr_constant_ranges; i++) {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> index 6a3b5dd..7c7a89e 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> @@ -338,7 +338,7 @@ struct ureg_src
>>   ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr, bool atomic);
>>
>>   struct ureg_src
>> -ureg_DECL_shared_memory(struct ureg_program *ureg);
>> +ureg_DECL_memory(struct ureg_program *ureg, unsigned memory_type);
>>
>>   static inline struct ureg_src
>>   ureg_imm4f( struct ureg_program *ureg,
>> 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 8683722..a8258af 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -860,7 +860,7 @@ public:
>>      std::vector<Resource> resources;
>>
>>      struct MemoryFile {
>> -      bool shared;
>> +      uint8_t mem_type; // TGSI_MEMORY_TYPE_*
>>      };
>>      std::vector<MemoryFile> memoryFiles;
>>
>> @@ -1218,7 +1218,7 @@ bool Source::scanDeclaration(const struct
>> tgsi_full_declaration *decl)
>>         break;
>>      case TGSI_FILE_MEMORY:
>>         for (i = first; i <= last; ++i)
>> -         memoryFiles[i].shared = decl->Declaration.Shared;
>> +         memoryFiles[i].mem_type = decl->Declaration.MemType;
>>         break;
>>      case TGSI_FILE_NULL:
>>      case TGSI_FILE_TEMPORARY:
>> @@ -1523,7 +1523,8 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>> idx, int c, uint32_t address)
>>
>>      sym->reg.fileIndex = fileIdx;
>>
>> -   if (tgsiFile == TGSI_FILE_MEMORY && code->memoryFiles[fileIdx].shared)
>> +   if (tgsiFile == TGSI_FILE_MEMORY &&
>> +       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>         sym->setFile(FILE_MEMORY_SHARED);
>>
>>      if (idx >= 0) {
>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
>> b/src/gallium/include/pipe/p_shader_tokens.h
>> index 9d4a96a..0b8d7fb 100644
>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>> @@ -117,6 +117,12 @@ enum tgsi_file_type {
>>   #define TGSI_CYLINDRICAL_WRAP_Z (1 << 2)
>>   #define TGSI_CYLINDRICAL_WRAP_W (1 << 3)
>>
>> +#define TGSI_MEMORY_TYPE_GLOBAL        0 /* GLSL global / OpenCL global
>>    */
>> +#define TGSI_MEMORY_TYPE_LOCAL         1 /* GLSL local  / OpenCL private
>>   */
>> +#define TGSI_MEMORY_TYPE_SHARED        2 /* GLSL shared / OpenCL local
>>   */
>> +#define TGSI_MEMORY_TYPE_INPUT         3 /* For OpenCL kernel input
>> params */
>>
>
> Note: I know little about how you are approaching the CL implementation for
> Nouveau...

The plan is to use a tgsi backend for llvm:

http://cgit.freedesktop.org/~jwrdegoede/llvm
http://cgit.freedesktop.org/~jwrdegoede/clang

And then have clover use that to feed tgsi to the mesa nouveau code.

> Should I expect to see an entry in here anywhere for the CONSTANT memory
> type?
 >
 > My GL knowledge is pretty basic. Would that map reasonably to something
 > like a GL UBO? Do we need an explicit type for CL constant buffers?

That is a good question, CONST has not really been on my radar
yet because it is kinda tricky. Since OpenCL is really C-like, the
best match for const is global mem, and then (optionally) map the
const buffers read-only. This is what amd is doing:

src/gallium/state_trackers/clover/llvm/invocation.cpp:

                // XXX: Correctly handle constant address space.  There is no
                // way for r600g to pass a handle for constant buffers back
                // to clover like it can for global buffers, so
                // creating constant arguments will break r600g.  For now,
                // continue treating constant buffers as global buffers
                // until we can come up with a way to create handles for
                // constant buffers.
                args.push_back(module::argument(module::argument::global,
                                                arg_api_size, target_size,
                                                target_align,
                                                module::argument::zero_ext));

The same more or less applies to nouveau. One day we would ideally map
const buffers to UBO-s but that is non trivial. I guess it would be
better for the llvm tgsi backend to do:

DCL MEMORY[42], CONST

and things like:

LOAD TEMP[0].x, MEMORY[42], IMM[0]

For const accesses so that if we later want to change things the TGSI
code already clearly differentiates between global and const accesses,
or maybe:

DCL MEMORY[42], GLOBAL, RO

Nah, I like:

DCL MEMORY[42], CONST

Better it is more consistent. Ilia, what is your take on this ?

The obvious implementation of this will eat another bit in
tgsi_declaration though.

Regards,

Hans


>
> --Aaron
>
>
>
>> +#define TGSI_MEMORY_TYPE_COUNT         4
>> +
>>   struct tgsi_declaration
>>   {
>>      unsigned Type        : 4;  /**< TGSI_TOKEN_TYPE_DECLARATION */
>> @@ -130,8 +136,8 @@ struct tgsi_declaration
>>      unsigned Local       : 1;  /**< optimize as subroutine local variable?
>> */
>>      unsigned Array       : 1;  /**< extra array info? */
>>      unsigned Atomic      : 1;  /**< atomic only? for TGSI_FILE_BUFFER */
>> -   unsigned Shared      : 1;  /**< shared storage for TGSI_FILE_MEMORY */
>> -   unsigned Padding     : 4;
>> +   unsigned MemType     : 2;  /**< TGSI_MEMORY_TYPE_x for
>> TGSI_FILE_MEMORY */
>> +   unsigned Padding     : 3;
>>   };
>>
>>   struct tgsi_declaration_range
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 26e463e..9b8bf8e 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -6281,7 +6281,7 @@ st_translate_program(
>>      }
>>
>>      if (program->use_shared_memory)
>> -      t->shared_memory = ureg_DECL_shared_memory(ureg);
>> +      t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
>>
>>      for (i = 0; i < program->shader->NumImages; i++) {
>>         if (program->images_used & (1 << i)) {
>> --
>> 2.7.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Hans de Goede]

Add support for clover / OpenCL kernel input parameters.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

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 a8258af..de0c72b 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
 
    sym->reg.fileIndex = fileIdx;
 
-   if (tgsiFile == TGSI_FILE_MEMORY &&
-       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
-      sym->setFile(FILE_MEMORY_SHARED);
+   if (tgsiFile == TGSI_FILE_MEMORY) {
+      switch (code->memoryFiles[fileIdx].mem_type) {
+      case TGSI_MEMORY_TYPE_SHARED:
+         sym->setFile(FILE_MEMORY_SHARED);
+         break;
+      case TGSI_MEMORY_TYPE_INPUT:
+         assert(prog->getType() == Program::TYPE_COMPUTE);
+         assert(idx == -1);
+         sym->setFile(FILE_SHADER_INPUT);
+         address += info->prop.cp.inputOffset;
+         break;
+      default:
+         assert(0); /* TODO: Add support for global and local memory */
+      }
+   }
 
    if (idx >= 0) {
       if (sym->reg.file == FILE_SHADER_INPUT)
-- 
2.7.2

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

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add support for clover / OpenCL kernel input parameters.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> 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 a8258af..de0c72b 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
>
>     sym->reg.fileIndex = fileIdx;
>
> -   if (tgsiFile == TGSI_FILE_MEMORY &&
> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
> -      sym->setFile(FILE_MEMORY_SHARED);
> +   if (tgsiFile == TGSI_FILE_MEMORY) {
> +      switch (code->memoryFiles[fileIdx].mem_type) {
> +      case TGSI_MEMORY_TYPE_SHARED:
> +         sym->setFile(FILE_MEMORY_SHARED);
> +         break;
> +      case TGSI_MEMORY_TYPE_INPUT:
> +         assert(prog->getType() == Program::TYPE_COMPUTE);
> +         assert(idx == -1);
> +         sym->setFile(FILE_SHADER_INPUT);
> +         address += info->prop.cp.inputOffset;

What's the idea here? i.e. what is the inputOffset, how is it set, and why?

  -ilia

> +         break;
> +      default:
> +         assert(0); /* TODO: Add support for global and local memory */
> +      }
> +   }
>
>     if (idx >= 0) {
>        if (sym->reg.file == FILE_SHADER_INPUT)
> --
> 2.7.2
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Samuel Pitoiset]

On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add support for clover / OpenCL kernel input parameters.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
>>   1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> 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 a8258af..de0c72b 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
>>
>>      sym->reg.fileIndex = fileIdx;
>>
>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>> -      sym->setFile(FILE_MEMORY_SHARED);
>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>> +      case TGSI_MEMORY_TYPE_SHARED:
>> +         sym->setFile(FILE_MEMORY_SHARED);
>> +         break;
>> +      case TGSI_MEMORY_TYPE_INPUT:
>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>> +         assert(idx == -1);
>> +         sym->setFile(FILE_SHADER_INPUT);
>> +         address += info->prop.cp.inputOffset;
>
> What's the idea here? i.e. what is the inputOffset, how is it set, and why?

I don't get the idea too, btw.

But prop.cp.inputOffset is only defined for compute on Kepler. It's the 
offset of input parameters in the screen->parm BO but as you already 
know, it is going to be removed because the idea is to use 
screen->uniform_bo instead. I'll do this change *after* the compute 
shaders support on Kepler.

>
>    -ilia
>
>> +         break;
>> +      default:
>> +         assert(0); /* TODO: Add support for global and local memory */
>> +      }
>> +   }
>>
>>      if (idx >= 0) {
>>         if (sym->reg.file == FILE_SHADER_INPUT)
>> --
>> 2.7.2
>>

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

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 10:27 AM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
>
> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>
>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Add support for clover / OpenCL kernel input parameters.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18
>>> +++++++++++++++---
>>>   1 file changed, 15 insertions(+), 3 deletions(-)
>>>
>>> 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 a8258af..de0c72b 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>>> idx, int c, uint32_t address)
>>>
>>>      sym->reg.fileIndex = fileIdx;
>>>
>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>> +         sym->setFile(FILE_MEMORY_SHARED);
>>> +         break;
>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>> +         assert(idx == -1);
>>> +         sym->setFile(FILE_SHADER_INPUT);
>>> +         address += info->prop.cp.inputOffset;
>>
>>
>> What's the idea here? i.e. what is the inputOffset, how is it set, and
>> why?
>
>
> I don't get the idea too, btw.
>
> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
> offset of input parameters in the screen->parm BO but as you already know,
> it is going to be removed because the idea is to use screen->uniform_bo
> instead. I'll do this change *after* the compute shaders support on Kepler.

Actually looks like it's only set for nv50 that I can see, shifting
things over by 0x10. It used to be reflected by getResourceBase, but
we broke that abstraction... might be nice to get it back somehow,
perhaps by sending more arguments down to getResourceBase? Either way,
that can be done later. This patch is

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Samuel Pitoiset]

On 03/10/2016 04:43 PM, Ilia Mirkin wrote:
> On Thu, Mar 10, 2016 at 10:27 AM, Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>>
>>
>> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>>
>>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Add support for clover / OpenCL kernel input parameters.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18
>>>> +++++++++++++++---
>>>>    1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> 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 a8258af..de0c72b 100644
>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>>>> idx, int c, uint32_t address)
>>>>
>>>>       sym->reg.fileIndex = fileIdx;
>>>>
>>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>>> +         sym->setFile(FILE_MEMORY_SHARED);
>>>> +         break;
>>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>>> +         assert(idx == -1);
>>>> +         sym->setFile(FILE_SHADER_INPUT);
>>>> +         address += info->prop.cp.inputOffset;
>>>
>>>
>>> What's the idea here? i.e. what is the inputOffset, how is it set, and
>>> why?
>>
>>
>> I don't get the idea too, btw.
>>
>> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
>> offset of input parameters in the screen->parm BO but as you already know,
>> it is going to be removed because the idea is to use screen->uniform_bo
>> instead. I'll do this change *after* the compute shaders support on Kepler.
>
> Actually looks like it's only set for nv50 that I can see, shifting
> things over by 0x10. It used to be reflected by getResourceBase, but
> we broke that abstraction... might be nice to get it back somehow,
> perhaps by sending more arguments down to getResourceBase? Either way,
> that can be done later. This patch is

Oh yes, I was confused with prop.cp.gridInfoBase on Kepler...

>
> Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
>

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

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Samuel Pitoiset]

Looks fine, except that you will need to lower FILE_SHADER_INPUT to 
FILE_MEMORY_SHARED for Tesla because input kernel parameters are located 
at s[0x10]. No need to do this for Fermi+ because it's already lowered 
to c0[]. Note that input kernel parameters will be probably sticked on 
c7[] after my changes but that doesn't change anything for you.

I already have a patch for the nv50 bits btw, maybe it's the right time 
to send it?

https://cgit.freedesktop.org/~hakzsam/mesa/commit/?h=compute&id=640d68009bcf93c1814cee0b1a12939cb85e5895

Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

On 03/10/2016 04:43 PM, Ilia Mirkin wrote:
> On Thu, Mar 10, 2016 at 10:27 AM, Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>>
>>
>> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>>
>>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Add support for clover / OpenCL kernel input parameters.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18
>>>> +++++++++++++++---
>>>>    1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> 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 a8258af..de0c72b 100644
>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>>>> idx, int c, uint32_t address)
>>>>
>>>>       sym->reg.fileIndex = fileIdx;
>>>>
>>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>>> +         sym->setFile(FILE_MEMORY_SHARED);
>>>> +         break;
>>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>>> +         assert(idx == -1);
>>>> +         sym->setFile(FILE_SHADER_INPUT);
>>>> +         address += info->prop.cp.inputOffset;
>>>
>>>
>>> What's the idea here? i.e. what is the inputOffset, how is it set, and
>>> why?
>>
>>
>> I don't get the idea too, btw.
>>
>> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
>> offset of input parameters in the screen->parm BO but as you already know,
>> it is going to be removed because the idea is to use screen->uniform_bo
>> instead. I'll do this change *after* the compute shaders support on Kepler.
>
> Actually looks like it's only set for nv50 that I can see, shifting
> things over by 0x10. It used to be reflected by getResourceBase, but
> we broke that abstraction... might be nice to get it back somehow,
> perhaps by sending more arguments down to getResourceBase? Either way,
> that can be done later. This patch is
>
> Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
>

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

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Hans de Goede]

Hi,

On 10-03-16 17:03, Samuel Pitoiset wrote:
> Looks fine, except that you will need to lower FILE_SHADER_INPUT to FILE_MEMORY_SHARED for Tesla because input kernel parameters are located at s[0x10].

Ok, but should this be done in nv50_ir_from_tgsi.cpp ? That feels like the wrong place to
handle this detail. Not sure where to do it otherwise though, and doing this later
may make the code more complicated.

 > No need to do this for Fermi+ because it's already lowered to c0[]. Note that input kernel parameters will be probably sticked on c7[] after my changes but that doesn't change anything for you.

Ack.

>
> I already have a patch for the nv50 bits btw, maybe it's the right time to send it?
>
> https://cgit.freedesktop.org/~hakzsam/mesa/commit/?h=compute&id=640d68009bcf93c1814cee0b1a12939cb85e5895

Ah I see that answers my question.

Yes I guess this is the right time to send it, although I've not really looked
at opencl for nv50 yet.

Regards,

Hans


>
> Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>
> On 03/10/2016 04:43 PM, Ilia Mirkin wrote:
>> On Thu, Mar 10, 2016 at 10:27 AM, Samuel Pitoiset
>> <samuel.pitoiset@gmail.com> wrote:
>>>
>>>
>>> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>>>
>>>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>> Add support for clover / OpenCL kernel input parameters.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>    .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18
>>>>> +++++++++++++++---
>>>>>    1 file changed, 15 insertions(+), 3 deletions(-)
>>>>>
>>>>> 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 a8258af..de0c72b 100644
>>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>>>>> idx, int c, uint32_t address)
>>>>>
>>>>>       sym->reg.fileIndex = fileIdx;
>>>>>
>>>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>>>> +         sym->setFile(FILE_MEMORY_SHARED);
>>>>> +         break;
>>>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>>>> +         assert(idx == -1);
>>>>> +         sym->setFile(FILE_SHADER_INPUT);
>>>>> +         address += info->prop.cp.inputOffset;
>>>>
>>>>
>>>> What's the idea here? i.e. what is the inputOffset, how is it set, and
>>>> why?
>>>
>>>
>>> I don't get the idea too, btw.
>>>
>>> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
>>> offset of input parameters in the screen->parm BO but as you already know,
>>> it is going to be removed because the idea is to use screen->uniform_bo
>>> instead. I'll do this change *after* the compute shaders support on Kepler.
>>
>> Actually looks like it's only set for nv50 that I can see, shifting
>> things over by 0x10. It used to be reflected by getResourceBase, but
>> we broke that abstraction... might be nice to get it back somehow,
>> perhaps by sending more arguments down to getResourceBase? Either way,
>> that can be done later. This patch is
>>
>> Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
>>
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Pierre Moreau]

[-- Attachment #1.1: Type: text/plain, Size: 3137 bytes --]

On 04:27 PM - Mar 10 2016, Samuel Pitoiset wrote:
> 
> 
> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
> >On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>Add support for clover / OpenCL kernel input parameters.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>  .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
> >>  1 file changed, 15 insertions(+), 3 deletions(-)
> >>
> >>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 a8258af..de0c72b 100644
> >>--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> >>+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> >>@@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
> >>
> >>     sym->reg.fileIndex = fileIdx;
> >>
> >>-   if (tgsiFile == TGSI_FILE_MEMORY &&
> >>-       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
> >>-      sym->setFile(FILE_MEMORY_SHARED);
> >>+   if (tgsiFile == TGSI_FILE_MEMORY) {
> >>+      switch (code->memoryFiles[fileIdx].mem_type) {
> >>+      case TGSI_MEMORY_TYPE_SHARED:
> >>+         sym->setFile(FILE_MEMORY_SHARED);

You might want to increment the address by at least
`info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,
then increment further by the input size. This input offset of 0x10 (or is it
0x20?) is due to the card sticking the size of a block and of the grid inside
`s[0x0..0x10]` (or maybe Nouveau is doing that, but I doubt it.). So even if
the user inputs end up somewhere else in memory, you most likely still don't
want to overwrite the grid information. This should be necessary only for Tesla
cards.

> >>+         break;
> >>+      case TGSI_MEMORY_TYPE_INPUT:
> >>+         assert(prog->getType() == Program::TYPE_COMPUTE);
> >>+         assert(idx == -1);
> >>+         sym->setFile(FILE_SHADER_INPUT);
> >>+         address += info->prop.cp.inputOffset;
> >
> >What's the idea here? i.e. what is the inputOffset, how is it set, and why?
> 
> I don't get the idea too, btw.
> 
> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
> offset of input parameters in the screen->parm BO but as you already know,
> it is going to be removed because the idea is to use screen->uniform_bo
> instead. I'll do this change *after* the compute shaders support on Kepler.

If I understand correctly, the goal is to have user inputs in a
`screen->uniform_bo`, and so for all generations?

Pierre


> 
> >
> >   -ilia
> >
> >>+         break;
> >>+      default:
> >>+         assert(0); /* TODO: Add support for global and local memory */
> >>+      }
> >>+   }
> >>
> >>     if (idx >= 0) {
> >>        if (sym->reg.file == FILE_SHADER_INPUT)
> >>--
> >>2.7.2
> >>
> 
> -- 
> -Samuel
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Nouveau] [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 11:03 AM, Pierre Moreau <pierre.morrow@free.fr> wrote:
> You might want to increment the address by at least
> `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,

There's a cp.sharedOffset just for that :) However it doesn't appear
to get set anywhere...
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Nouveau] [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Pierre Moreau]

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

On 11:05 AM - Mar 10 2016, Ilia Mirkin wrote:
> On Thu, Mar 10, 2016 at 11:03 AM, Pierre Moreau <pierre.morrow@free.fr> wrote:
> > You might want to increment the address by at least
> > `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,
> 
> There's a cp.sharedOffset just for that :) However it doesn't appear
> to get set anywhere...

Oh really?! I completely missed that one… Well, I have some changes to make on
my own code then! :-D Thanks for pointing that out!

Pierre


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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [Nouveau] [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Samuel Pitoiset]

On 03/10/2016 05:03 PM, Pierre Moreau wrote:
> On 04:27 PM - Mar 10 2016, Samuel Pitoiset wrote:
>>
>>
>> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Add support for clover / OpenCL kernel input parameters.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
>>>>   1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> 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 a8258af..de0c72b 100644
>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
>>>>
>>>>      sym->reg.fileIndex = fileIdx;
>>>>
>>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>>> +         sym->setFile(FILE_MEMORY_SHARED);
>
> You might want to increment the address by at least
> `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,
> then increment further by the input size. This input offset of 0x10 (or is it
> 0x20?) is due to the card sticking the size of a block and of the grid inside
> `s[0x0..0x10]` (or maybe Nouveau is doing that, but I doubt it.). So even if
> the user inputs end up somewhere else in memory, you most likely still don't
> want to overwrite the grid information. This should be necessary only for Tesla
> cards.

cf. my previous comment. :-)

>
>>>> +         break;
>>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>>> +         assert(idx == -1);
>>>> +         sym->setFile(FILE_SHADER_INPUT);
>>>> +         address += info->prop.cp.inputOffset;
>>>
>>> What's the idea here? i.e. what is the inputOffset, how is it set, and why?
>>
>> I don't get the idea too, btw.
>>
>> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
>> offset of input parameters in the screen->parm BO but as you already know,
>> it is going to be removed because the idea is to use screen->uniform_bo
>> instead. I'll do this change *after* the compute shaders support on Kepler.
>
> If I understand correctly, the goal is to have user inputs in a
> `screen->uniform_bo`, and so for all generations?

Sure for fermi, and probably for Tesla.

>
> Pierre
>
>
>>
>>>
>>>    -ilia
>>>
>>>> +         break;
>>>> +      default:
>>>> +         assert(0); /* TODO: Add support for global and local memory */
>>>> +      }
>>>> +   }
>>>>
>>>>      if (idx >= 0) {
>>>>         if (sym->reg.file == FILE_SHADER_INPUT)
>>>> --
>>>> 2.7.2
>>>>
>>
>> --
>> -Samuel
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau

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

Re: [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 11:05 AM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>> If I understand correctly, the goal is to have user inputs in a
>> `screen->uniform_bo`, and so for all generations?
>
> Sure for fermi, and probably for Tesla.

I think continuing to use the USER_PARAMS or whatever mechanism on
telsa makes sense. That's why I agreed to keep the MEMORY, INPUT
concept.

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