Re: [Mesa-dev] [PATCH v2 07/25] mesa: refuse to compile SPIR-V shaders or link mixed shaders

[Ian Romanick]

On 11/30/2017 09:28 AM, Eduardo Lima Mitev wrote:
> From: Nicolai Hähnle 
> 
> Note that gl_shader::CompileStatus will also indicate whether a shader
> has been successfully specialized.
> 
> v2: Use the 'spirv_data' member of gl_shader to know if it is a SPIR-V
>shader, instead of a dedicated flag. (Timothy Arceri)
> ---
>  src/mesa/main/shaderapi.c   | 12 
>  src/mesa/program/ir_to_mesa.cpp | 17 -
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 3ac1419b7ee..251c876ada8 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1092,6 +1092,18 @@ _mesa_compile_shader(struct gl_context *ctx, struct 
> gl_shader *sh)
> if (!sh)
>return;
>  
> +   /* The GL_ARB_gl_spirv spec says:
> +*
> +*"Add a new error for the CompileShader command:
> +*
> +*  An INVALID_OPERATION error is generated if the SPIR_V_BINARY_ARB
> +*  state of  is TRUE."
> +*/
> +   if (sh->spirv_data) {
> +  _mesa_error(ctx, GL_INVALID_OPERATION, "glCompileShader(SPIR-V)");
> +  return;
> +   }
> +
> if (!sh->Source) {
>/* If the user called glCompileShader without first calling
> * glShaderSource, we should fail to compile, but not raise a GL_ERROR.
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index aa8b6d7084b..047f5b38f71 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3077,6 +3077,7 @@ void
>  _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program 
> *prog)
>  {
> unsigned int i;
> +   GLboolean spirv;

Use bool.  With that fixed, this patch is

Reviewed-by: Ian Romanick 

>  
> _mesa_clear_shader_program_data(ctx, prog);
>  
> @@ -3086,7 +3087,21 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct 
> gl_shader_program *prog)
>  
> for (i = 0; i < prog->NumShaders; i++) {
>if (!prog->Shaders[i]->CompileStatus) {
> -  linker_error(prog, "linking with uncompiled shader");
> +  linker_error(prog, "linking with uncompiled/unspecialized shader");
> +  }
> +
> +  if (!i) {
> + spirv = (prog->Shaders[i]->spirv_data != NULL);
> +  } else if (spirv && !prog->Shaders[i]->spirv_data) {
> + /* The GL_ARB_gl_spirv spec adds a new bullet point to the list of
> +  * reasons LinkProgram can fail:
> +  *
> +  *"All the shader objects attached to  do not have the
> +  * same value for the SPIR_V_BINARY_ARB state."
> +  */
> + linker_error(prog,
> +  "not all attached shaders have the same "
> +  "SPIR_V_BINARY_ARB state");
>}
> }
>  
> 

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

[Mesa-dev] [PATCH v2 07/25] mesa: refuse to compile SPIR-V shaders or link mixed shaders

[Eduardo Lima Mitev]

From: Nicolai Hähnle 

Note that gl_shader::CompileStatus will also indicate whether a shader
has been successfully specialized.

v2: Use the 'spirv_data' member of gl_shader to know if it is a SPIR-V
   shader, instead of a dedicated flag. (Timothy Arceri)
---
 src/mesa/main/shaderapi.c   | 12 
 src/mesa/program/ir_to_mesa.cpp | 17 -
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 3ac1419b7ee..251c876ada8 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1092,6 +1092,18 @@ _mesa_compile_shader(struct gl_context *ctx, struct 
gl_shader *sh)
if (!sh)
   return;
 
+   /* The GL_ARB_gl_spirv spec says:
+*
+*"Add a new error for the CompileShader command:
+*
+*  An INVALID_OPERATION error is generated if the SPIR_V_BINARY_ARB
+*  state of  is TRUE."
+*/
+   if (sh->spirv_data) {
+  _mesa_error(ctx, GL_INVALID_OPERATION, "glCompileShader(SPIR-V)");
+  return;
+   }
+
if (!sh->Source) {
   /* If the user called glCompileShader without first calling
* glShaderSource, we should fail to compile, but not raise a GL_ERROR.
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index aa8b6d7084b..047f5b38f71 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3077,6 +3077,7 @@ void
 _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
 {
unsigned int i;
+   GLboolean spirv;
 
_mesa_clear_shader_program_data(ctx, prog);
 
@@ -3086,7 +3087,21 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
 
for (i = 0; i < prog->NumShaders; i++) {
   if (!prog->Shaders[i]->CompileStatus) {
-linker_error(prog, "linking with uncompiled shader");
+linker_error(prog, "linking with uncompiled/unspecialized shader");
+  }
+
+  if (!i) {
+ spirv = (prog->Shaders[i]->spirv_data != NULL);
+  } else if (spirv && !prog->Shaders[i]->spirv_data) {
+ /* The GL_ARB_gl_spirv spec adds a new bullet point to the list of
+  * reasons LinkProgram can fail:
+  *
+  *"All the shader objects attached to  do not have the
+  * same value for the SPIR_V_BINARY_ARB state."
+  */
+ linker_error(prog,
+  "not all attached shaders have the same "
+  "SPIR_V_BINARY_ARB state");
   }
}
 
-- 
2.15.0

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