[03/24] mesa: refuse to compile SPIR-V shaders or link mixed shaders

Submitted by Eduardo Lima Mitev on Nov. 15, 2017, 1:22 p.m.

Details

Message ID 20171115132227.23829-4-elima@igalia.com
State New
Headers show
Series "Initial gl_spirv and spirv_extensions support in Mesa and i965" ( rev: 4 3 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Eduardo Lima Mitev Nov. 15, 2017, 1:22 p.m.
From: Nicolai Hähnle <nicolai.haehnle@amd.com>

Note that gl_shader::CompileStatus will also indicate whether a shader
has been successfully specialized.
---
 src/mesa/main/shaderapi.c       | 12 ++++++++++++
 src/mesa/program/ir_to_mesa.cpp | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 5db335b695b..9090bee9fb0 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1081,6 +1081,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 <shader> is TRUE."
+    */
+   if (sh->SpirVBinary) {
+      _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 327fd61d422..ddc1705097d 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3064,6 +3064,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);
 
@@ -3073,7 +3074,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]->SpirVBinary;
+      } else if (spirv != prog->Shaders[i]->SpirVBinary) {
+         /* 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 <program> 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");
       }
    }
 

Comments

This patch is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

On 11/15/2017 05:22 AM, Eduardo Lima Mitev wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
> 
> Note that gl_shader::CompileStatus will also indicate whether a shader
> has been successfully specialized.
> ---
>  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 5db335b695b..9090bee9fb0 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1081,6 +1081,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 <shader> is TRUE."
> +    */
> +   if (sh->SpirVBinary) {
> +      _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 327fd61d422..ddc1705097d 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3064,6 +3064,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);
>  
> @@ -3073,7 +3074,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]->SpirVBinary;
> +      } else if (spirv != prog->Shaders[i]->SpirVBinary) {
> +         /* 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 <program> 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");
>        }
>     }
>  
>