Re: [Mesa-dev] [PATCH v2 16/21] nir/linker: Add nir_build_program_resource_list()

2018-05-17 Thread Timothy Arceri 



On 15/05/18 01:05, Alejandro Piñeiro wrote:

On 14/05/18 01:26, Timothy Arceri wrote:

On 12/05/18 19:40, Alejandro Piñeiro wrote:

From: Eduardo Lima Mitev 

This function is equivalent to the linker.cpp
build_program_resource_list() but will extract the resources from NIR
shaders instead.

For now, only uniforms and program inputs are implemented.

v2: move from compiler/nir to compiler/glsl (Timothy Arceri)

Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
   src/compiler/Makefile.sources |  1 +
   src/compiler/glsl/gl_nir_linker.c | 93
+++
   src/compiler/glsl/gl_nir_linker.h |  3 ++
   src/compiler/glsl/meson.build |  1 +
   4 files changed, 98 insertions(+)
   create mode 100644 src/compiler/glsl/gl_nir_linker.c

diff --git a/src/compiler/Makefile.sources
b/src/compiler/Makefile.sources
index 8104dd32002..409252ee587 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -30,6 +30,7 @@ LIBGLSL_FILES = \
   glsl/gl_nir_lower_samplers_as_deref.c \
   glsl/gl_nir_link_uniform_initializers.c \
   glsl/gl_nir_link_uniforms.c \
+    glsl/gl_nir_linker.c \
   glsl/gl_nir_linker.h \
   glsl/gl_nir.h \
   glsl/glsl_parser_extras.cpp \
diff --git a/src/compiler/glsl/gl_nir_linker.c
b/src/compiler/glsl/gl_nir_linker.c
new file mode 100644
index 000..155d94ef966
--- /dev/null
+++ b/src/compiler/glsl/gl_nir_linker.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a
+ * copy of this software and associated documentation files (the
"Software"),
+ * to deal in the Software without restriction, including without
limitation
+ * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom
the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including
the next
+ * paragraph) shall be included in all copies or substantial
portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "nir.h"
+#include "gl_nir_linker.h"
+#include "linker_util.h"
+#include "main/mtypes.h"
+#include "ir_uniform.h" /* for gl_uniform_storage */
+
+/* This file included general link methods, using NIR, instead of IR as
+ * the counter-part glsl/linker.cpp
+ *
+ * Also note that this is tailored for ARB_gl_spirv needs and
particularities
+ */
+
+void
+nir_build_program_resource_list(struct gl_context *ctx,
+    struct gl_shader_program *prog)
+{
+   /* Rebuild resource list. */
+   if (prog->data->ProgramResourceList) {
+  ralloc_free(prog->data->ProgramResourceList);
+  prog->data->ProgramResourceList = NULL;
+  prog->data->NumProgramResourceList = 0;
+   }
+
+   struct set *resource_set = _mesa_set_create(NULL,
+   _mesa_hash_pointer,
+
_mesa_key_pointer_equal);
+
+   /* Add uniforms
+    *
+    * Here, it is expected that nir_link_uniforms() has already been
+    * called, so that UniformStorage table is already available.
+    */
+   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+  struct gl_uniform_storage *uniform =
>data->UniformStorage[i];
+
+  if (!link_util_add_program_resource(prog, resource_set,
GL_UNIFORM, uniform,
+
uniform->active_shader_mask)) {
+ return;
+  }
+   }
+
+   /* Add inputs */
+   struct gl_linked_shader *sh =
prog->_LinkedShaders[MESA_SHADER_VERTEX];
+   if (sh) {
+  nir_shader *nir = sh->Program->nir;
+  assert(nir);
+
+  nir_foreach_variable(var, >inputs) {
+ struct gl_shader_variable *sh_var =
+    rzalloc(prog, struct gl_shader_variable);
+
+ /* ARB_gl_spirv: names are considered optional debug info,
so the linker
+  * needs to work without them, and returning them is
optional. For
+  * simplicity we ignore names.
+  */
+ sh_var->name = NULL;
+ sh_var->type = var->type;
+ sh_var->location = var->data.location;


This doesn't look right to me. Don't we need to handle structs, ifcs
and arrays just like in the glsl code?



+
+ /* @TODO: Fill in the rest of gl_shader_variable data. */
+
+ if (!link_util_add_program_resource(prog, resource_set,
GL_PROGRAM_INPUT,
+   

Re: [Mesa-dev] [PATCH v2 16/21] nir/linker: Add nir_build_program_resource_list()

2018-05-14 Thread Alejandro Piñeiro 
On 14/05/18 01:26, Timothy Arceri wrote:
> On 12/05/18 19:40, Alejandro Piñeiro wrote:
>> From: Eduardo Lima Mitev 
>>
>> This function is equivalent to the linker.cpp
>> build_program_resource_list() but will extract the resources from NIR
>> shaders instead.
>>
>> For now, only uniforms and program inputs are implemented.
>>
>> v2: move from compiler/nir to compiler/glsl (Timothy Arceri)
>>
>> Signed-off-by: Eduardo Lima 
>> Signed-off-by: Alejandro Piñeiro 
>> ---
>>   src/compiler/Makefile.sources |  1 +
>>   src/compiler/glsl/gl_nir_linker.c | 93
>> +++
>>   src/compiler/glsl/gl_nir_linker.h |  3 ++
>>   src/compiler/glsl/meson.build |  1 +
>>   4 files changed, 98 insertions(+)
>>   create mode 100644 src/compiler/glsl/gl_nir_linker.c
>>
>> diff --git a/src/compiler/Makefile.sources
>> b/src/compiler/Makefile.sources
>> index 8104dd32002..409252ee587 100644
>> --- a/src/compiler/Makefile.sources
>> +++ b/src/compiler/Makefile.sources
>> @@ -30,6 +30,7 @@ LIBGLSL_FILES = \
>>   glsl/gl_nir_lower_samplers_as_deref.c \
>>   glsl/gl_nir_link_uniform_initializers.c \
>>   glsl/gl_nir_link_uniforms.c \
>> +    glsl/gl_nir_linker.c \
>>   glsl/gl_nir_linker.h \
>>   glsl/gl_nir.h \
>>   glsl/glsl_parser_extras.cpp \
>> diff --git a/src/compiler/glsl/gl_nir_linker.c
>> b/src/compiler/glsl/gl_nir_linker.c
>> new file mode 100644
>> index 000..155d94ef966
>> --- /dev/null
>> +++ b/src/compiler/glsl/gl_nir_linker.c
>> @@ -0,0 +1,93 @@
>> +/*
>> + * Copyright © 2018 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> the next
>> + * paragraph) shall be included in all copies or substantial
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#include "nir.h"
>> +#include "gl_nir_linker.h"
>> +#include "linker_util.h"
>> +#include "main/mtypes.h"
>> +#include "ir_uniform.h" /* for gl_uniform_storage */
>> +
>> +/* This file included general link methods, using NIR, instead of IR as
>> + * the counter-part glsl/linker.cpp
>> + *
>> + * Also note that this is tailored for ARB_gl_spirv needs and
>> particularities
>> + */
>> +
>> +void
>> +nir_build_program_resource_list(struct gl_context *ctx,
>> +    struct gl_shader_program *prog)
>> +{
>> +   /* Rebuild resource list. */
>> +   if (prog->data->ProgramResourceList) {
>> +  ralloc_free(prog->data->ProgramResourceList);
>> +  prog->data->ProgramResourceList = NULL;
>> +  prog->data->NumProgramResourceList = 0;
>> +   }
>> +
>> +   struct set *resource_set = _mesa_set_create(NULL,
>> +   _mesa_hash_pointer,
>> +  
>> _mesa_key_pointer_equal);
>> +
>> +   /* Add uniforms
>> +    *
>> +    * Here, it is expected that nir_link_uniforms() has already been
>> +    * called, so that UniformStorage table is already available.
>> +    */
>> +   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
>> +  struct gl_uniform_storage *uniform =
>> >data->UniformStorage[i];
>> +
>> +  if (!link_util_add_program_resource(prog, resource_set,
>> GL_UNIFORM, uniform,
>> + 
>> uniform->active_shader_mask)) {
>> + return;
>> +  }
>> +   }
>> +
>> +   /* Add inputs */
>> +   struct gl_linked_shader *sh =
>> prog->_LinkedShaders[MESA_SHADER_VERTEX];
>> +   if (sh) {
>> +  nir_shader *nir = sh->Program->nir;
>> +  assert(nir);
>> +
>> +  nir_foreach_variable(var, >inputs) {
>> + struct gl_shader_variable *sh_var =
>> +    rzalloc(prog, struct gl_shader_variable);
>> +
>> + /* ARB_gl_spirv: names are considered optional debug info,
>> so the linker
>> +  * needs to work without them, and returning them is
>> optional. For
>> +  * simplicity we ignore

Re: [Mesa-dev] [PATCH v2 16/21] nir/linker: Add nir_build_program_resource_list()

2018-05-13 Thread Timothy Arceri 

On 12/05/18 19:40, Alejandro Piñeiro wrote:

From: Eduardo Lima Mitev 

This function is equivalent to the linker.cpp
build_program_resource_list() but will extract the resources from NIR
shaders instead.

For now, only uniforms and program inputs are implemented.

v2: move from compiler/nir to compiler/glsl (Timothy Arceri)

Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
  src/compiler/Makefile.sources |  1 +
  src/compiler/glsl/gl_nir_linker.c | 93 +++
  src/compiler/glsl/gl_nir_linker.h |  3 ++
  src/compiler/glsl/meson.build |  1 +
  4 files changed, 98 insertions(+)
  create mode 100644 src/compiler/glsl/gl_nir_linker.c

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 8104dd32002..409252ee587 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -30,6 +30,7 @@ LIBGLSL_FILES = \
glsl/gl_nir_lower_samplers_as_deref.c \
glsl/gl_nir_link_uniform_initializers.c \
glsl/gl_nir_link_uniforms.c \
+   glsl/gl_nir_linker.c \
glsl/gl_nir_linker.h \
glsl/gl_nir.h \
glsl/glsl_parser_extras.cpp \
diff --git a/src/compiler/glsl/gl_nir_linker.c 
b/src/compiler/glsl/gl_nir_linker.c
new file mode 100644
index 000..155d94ef966
--- /dev/null
+++ b/src/compiler/glsl/gl_nir_linker.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "nir.h"
+#include "gl_nir_linker.h"
+#include "linker_util.h"
+#include "main/mtypes.h"
+#include "ir_uniform.h" /* for gl_uniform_storage */
+
+/* This file included general link methods, using NIR, instead of IR as
+ * the counter-part glsl/linker.cpp
+ *
+ * Also note that this is tailored for ARB_gl_spirv needs and particularities
+ */
+
+void
+nir_build_program_resource_list(struct gl_context *ctx,
+struct gl_shader_program *prog)
+{
+   /* Rebuild resource list. */
+   if (prog->data->ProgramResourceList) {
+  ralloc_free(prog->data->ProgramResourceList);
+  prog->data->ProgramResourceList = NULL;
+  prog->data->NumProgramResourceList = 0;
+   }
+
+   struct set *resource_set = _mesa_set_create(NULL,
+   _mesa_hash_pointer,
+   _mesa_key_pointer_equal);
+
+   /* Add uniforms
+*
+* Here, it is expected that nir_link_uniforms() has already been
+* called, so that UniformStorage table is already available.
+*/
+   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+  struct gl_uniform_storage *uniform = >data->UniformStorage[i];
+
+  if (!link_util_add_program_resource(prog, resource_set, GL_UNIFORM, 
uniform,
+  uniform->active_shader_mask)) {
+ return;
+  }
+   }
+
+   /* Add inputs */
+   struct gl_linked_shader *sh = prog->_LinkedShaders[MESA_SHADER_VERTEX];
+   if (sh) {
+  nir_shader *nir = sh->Program->nir;
+  assert(nir);
+
+  nir_foreach_variable(var, >inputs) {
+ struct gl_shader_variable *sh_var =
+rzalloc(prog, struct gl_shader_variable);
+
+ /* ARB_gl_spirv: names are considered optional debug info, so the 
linker
+  * needs to work without them, and returning them is optional. For
+  * simplicity we ignore names.
+  */
+ sh_var->name = NULL;
+ sh_var->type = var->type;
+ sh_var->location = var->data.location;


This doesn't look right to me. Don't we need to handle structs, ifcs and 
arrays just like in the glsl code?




+
+ /* @TODO: Fill in the rest of gl_shader_variable data. */
+
+ if (!link_util_add_program_resource(prog, resource_set, 
GL_PROGRAM_INPUT,
+

[Mesa-dev] [PATCH v2 16/21] nir/linker: Add nir_build_program_resource_list()

2018-05-12 Thread Alejandro Piñeiro 
From: Eduardo Lima Mitev 

This function is equivalent to the linker.cpp
build_program_resource_list() but will extract the resources from NIR
shaders instead.

For now, only uniforms and program inputs are implemented.

v2: move from compiler/nir to compiler/glsl (Timothy Arceri)

Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
 src/compiler/Makefile.sources |  1 +
 src/compiler/glsl/gl_nir_linker.c | 93 +++
 src/compiler/glsl/gl_nir_linker.h |  3 ++
 src/compiler/glsl/meson.build |  1 +
 4 files changed, 98 insertions(+)
 create mode 100644 src/compiler/glsl/gl_nir_linker.c

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 8104dd32002..409252ee587 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -30,6 +30,7 @@ LIBGLSL_FILES = \
glsl/gl_nir_lower_samplers_as_deref.c \
glsl/gl_nir_link_uniform_initializers.c \
glsl/gl_nir_link_uniforms.c \
+   glsl/gl_nir_linker.c \
glsl/gl_nir_linker.h \
glsl/gl_nir.h \
glsl/glsl_parser_extras.cpp \
diff --git a/src/compiler/glsl/gl_nir_linker.c 
b/src/compiler/glsl/gl_nir_linker.c
new file mode 100644
index 000..155d94ef966
--- /dev/null
+++ b/src/compiler/glsl/gl_nir_linker.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "nir.h"
+#include "gl_nir_linker.h"
+#include "linker_util.h"
+#include "main/mtypes.h"
+#include "ir_uniform.h" /* for gl_uniform_storage */
+
+/* This file included general link methods, using NIR, instead of IR as
+ * the counter-part glsl/linker.cpp
+ *
+ * Also note that this is tailored for ARB_gl_spirv needs and particularities
+ */
+
+void
+nir_build_program_resource_list(struct gl_context *ctx,
+struct gl_shader_program *prog)
+{
+   /* Rebuild resource list. */
+   if (prog->data->ProgramResourceList) {
+  ralloc_free(prog->data->ProgramResourceList);
+  prog->data->ProgramResourceList = NULL;
+  prog->data->NumProgramResourceList = 0;
+   }
+
+   struct set *resource_set = _mesa_set_create(NULL,
+   _mesa_hash_pointer,
+   _mesa_key_pointer_equal);
+
+   /* Add uniforms
+*
+* Here, it is expected that nir_link_uniforms() has already been
+* called, so that UniformStorage table is already available.
+*/
+   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+  struct gl_uniform_storage *uniform = >data->UniformStorage[i];
+
+  if (!link_util_add_program_resource(prog, resource_set, GL_UNIFORM, 
uniform,
+  uniform->active_shader_mask)) {
+ return;
+  }
+   }
+
+   /* Add inputs */
+   struct gl_linked_shader *sh = prog->_LinkedShaders[MESA_SHADER_VERTEX];
+   if (sh) {
+  nir_shader *nir = sh->Program->nir;
+  assert(nir);
+
+  nir_foreach_variable(var, >inputs) {
+ struct gl_shader_variable *sh_var =
+rzalloc(prog, struct gl_shader_variable);
+
+ /* ARB_gl_spirv: names are considered optional debug info, so the 
linker
+  * needs to work without them, and returning them is optional. For
+  * simplicity we ignore names.
+  */
+ sh_var->name = NULL;
+ sh_var->type = var->type;
+ sh_var->location = var->data.location;
+
+ /* @TODO: Fill in the rest of gl_shader_variable data. */
+
+ if (!link_util_add_program_resource(prog, resource_set, 
GL_PROGRAM_INPUT,
+ sh_var, 1 << MESA_SHADER_VERTEX)) 
{
+return;
+ }
+  }
+   }
+
+   _mesa_set_destroy(resource_set, NULL);
+}
diff --git