Re: [Mesa-dev] [PATCH v3 22/25] mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program

2017-12-03 Thread Timothy Arceri



On 01/12/17 18:15, Eduardo Lima Mitev wrote:

v2: Bail out if we see more that one shader for the same stage, and add
a corresponding comment. (Timothy Arceri)
---
  src/mesa/main/glspirv.c | 61 ++---
  1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e533853f7fa..0934ceccbb3 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
  #include "compiler/nir/nir.h"
  #include "compiler/spirv/nir_spirv.h"
  
+#include "program/program.h"

+
  #include "util/u_atomic.h"
  
  void

@@ -104,14 +106,67 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
 }
  }
  
+/**

+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
  void
  _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program 
*prog)
  {
-   /* @TODO: This is a placeholder for the equivalent of
-* compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
-*/
 prog->data->LinkStatus = linking_success;
 prog->data->Validated = false;
+
+   for (unsigned i = 0; i < prog->NumShaders; i++) {
+  struct gl_shader *shader = prog->Shaders[i];
+  gl_shader_stage shader_type = shader->Stage;
+
+  /* We only support one shader per stage. The gl_spirv spec doesn't seem
+   * to prevent this, but the way the API is designed, requiring all 
shaders
+   * to be specialized with an entry point, makes supporting this quite
+   * undefined.
+   */
+  if (prog->_LinkedShaders[shader_type]) {


We should probably report an error here too right? Otherwise we would 
fail to link and the user would have no idea why.



+ prog->data->LinkStatus = linking_failure;
+ return;
+  }
+
+  assert(shader->spirv_data);
+
+  struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+  linked->Stage = shader_type;
+
+  /* Create program and attach it to the linked shader */
+  struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+_mesa_shader_stage_to_program(shader_type),
+prog->Name, false);
+  if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+  }
+
+  _mesa_reference_shader_program_data(ctx,
+  _prog->sh.data,
+  prog->data);
+
+  /* Don't use _mesa_reference_program() just take ownership */
+  linked->Program = gl_prog;
+
+  /* Reference the SPIR-V data from shader to the linked shader */
+  _mesa_shader_spirv_data_reference(>spirv_data,
+shader->spirv_data);
+
+  prog->_LinkedShaders[shader_type] = linked;
+  prog->data->linked_stages |= 1 << shader_type;
+   }
  }
  
  void GLAPIENTRY



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


[Mesa-dev] [PATCH v3 22/25] mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program

2017-11-30 Thread Eduardo Lima Mitev
v2: Bail out if we see more that one shader for the same stage, and add
   a corresponding comment. (Timothy Arceri)
---
 src/mesa/main/glspirv.c | 61 ++---
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e533853f7fa..0934ceccbb3 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
 #include "compiler/nir/nir.h"
 #include "compiler/spirv/nir_spirv.h"
 
+#include "program/program.h"
+
 #include "util/u_atomic.h"
 
 void
@@ -104,14 +106,67 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
 }
 
+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
 void
 _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program 
*prog)
 {
-   /* @TODO: This is a placeholder for the equivalent of
-* compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
-*/
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+   for (unsigned i = 0; i < prog->NumShaders; i++) {
+  struct gl_shader *shader = prog->Shaders[i];
+  gl_shader_stage shader_type = shader->Stage;
+
+  /* We only support one shader per stage. The gl_spirv spec doesn't seem
+   * to prevent this, but the way the API is designed, requiring all 
shaders
+   * to be specialized with an entry point, makes supporting this quite
+   * undefined.
+   */
+  if (prog->_LinkedShaders[shader_type]) {
+ prog->data->LinkStatus = linking_failure;
+ return;
+  }
+
+  assert(shader->spirv_data);
+
+  struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+  linked->Stage = shader_type;
+
+  /* Create program and attach it to the linked shader */
+  struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+_mesa_shader_stage_to_program(shader_type),
+prog->Name, false);
+  if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+  }
+
+  _mesa_reference_shader_program_data(ctx,
+  _prog->sh.data,
+  prog->data);
+
+  /* Don't use _mesa_reference_program() just take ownership */
+  linked->Program = gl_prog;
+
+  /* Reference the SPIR-V data from shader to the linked shader */
+  _mesa_shader_spirv_data_reference(>spirv_data,
+shader->spirv_data);
+
+  prog->_LinkedShaders[shader_type] = linked;
+  prog->data->linked_stages |= 1 << shader_type;
+   }
 }
 
 void GLAPIENTRY
-- 
2.15.0

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