mesa/spirv: Provide more specific error message for glSpecializeShader()

Distinguish between the "entry point not found" and "parsing error"
cases in the error text.  For consistency, identify the unhandled
specialization index case as part of the verification function.

The verification function was renamed to make clearer its scope and
what module it belongs.

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22976>

diff --git a/src/compiler/spirv/gl_spirv.c b/src/compiler/spirv/gl_spirv.c
index 9b5e351..065a4f2 100644 (file)
--- a/src/compiler/spirv/gl_spirv.c
+++ b/src/compiler/spirv/gl_spirv.c
@@ -226,10 +226,11 @@ vtn_validate_handle_constant_instruction(struct vtn_builder *b, SpvOp opcode,
  * would need to trigger the specific errors.
  *
  */
-bool
-gl_spirv_validation(const uint32_t *words, size_t word_count,
-                    struct nir_spirv_specialization *spec, unsigned num_spec,
-                    gl_shader_stage stage, const char *entry_point_name)
+enum spirv_verify_result
+spirv_verify_gl_specialization_constants(
+   const uint32_t *words, size_t word_count,
+   struct nir_spirv_specialization *spec, unsigned num_spec,
+   gl_shader_stage stage, const char *entry_point_name)
 {
    /* vtn_warn/vtn_log uses debug.func. Setting a null to prevent crash. Not
     * need to print the warnings now, would be done later, on the real
@@ -248,7 +249,7 @@ gl_spirv_validation(const uint32_t *words, size_t word_count,
    /* See also _vtn_fail() */
    if (vtn_setjmp(b->fail_jump)) {
       ralloc_free(b);
-      return false;
+      return SPIRV_VERIFY_PARSER_ERROR;
    }
 
    /* Skip the SPIR-V header, handled at vtn_create_builder */
@@ -260,7 +261,7 @@ gl_spirv_validation(const uint32_t *words, size_t word_count,
 
    if (b->entry_point == NULL) {
       ralloc_free(b);
-      return false;
+      return SPIRV_VERIFY_ENTRY_POINT_NOT_FOUND;
    }
 
    b->specializations = spec;
@@ -274,6 +275,11 @@ gl_spirv_validation(const uint32_t *words, size_t word_count,
 
    ralloc_free(b);
 
-   return true;
+   for (unsigned i = 0; i < num_spec; i++) {
+      if (!spec[i].defined_on_module)
+         return SPIRV_VERIFY_UNKNOWN_SPEC_INDEX;
+   }
+
+   return SPIRV_VERIFY_OK;
 }
 

diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index eb7550d..debcf9a 100644 (file)
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -124,9 +124,17 @@ struct spirv_to_nir_options {
    bool skip_os_break_in_debug_build;
 };
 
-bool gl_spirv_validation(const uint32_t *words, size_t word_count,
-                         struct nir_spirv_specialization *spec, unsigned num_spec,
-                         gl_shader_stage stage, const char *entry_point_name);
+enum spirv_verify_result {
+   SPIRV_VERIFY_OK = 0,
+   SPIRV_VERIFY_PARSER_ERROR = 1,
+   SPIRV_VERIFY_ENTRY_POINT_NOT_FOUND = 2,
+   SPIRV_VERIFY_UNKNOWN_SPEC_INDEX = 3,
+};
+
+enum spirv_verify_result spirv_verify_gl_specialization_constants(
+   const uint32_t *words, size_t word_count,
+   struct nir_spirv_specialization *spec, unsigned num_spec,
+   gl_shader_stage stage, const char *entry_point_name);
 
 nir_shader *spirv_to_nir(const uint32_t *words, size_t word_count,
                          struct nir_spirv_specialization *specializations,

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 48649a4..949962a 100644 (file)
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -329,7 +329,6 @@ _mesa_SpecializeShaderARB(GLuint shader,
 {
    GET_CURRENT_CONTEXT(ctx);
    struct gl_shader *sh;
-   bool has_entry_point;
    struct nir_spirv_specialization *spec_entries = NULL;
 
    if (!ctx->Extensions.ARB_gl_spirv) {
@@ -384,27 +383,35 @@ _mesa_SpecializeShaderARB(GLuint shader,
       spec_entries[i].defined_on_module = false;
    }
 
-   has_entry_point =
-      gl_spirv_validation((uint32_t *)&spirv_data->SpirVModule->Binary[0],
-                          spirv_data->SpirVModule->Length / 4,
-                          spec_entries, numSpecializationConstants,
-                          sh->Stage, pEntryPoint);
+   enum spirv_verify_result r = spirv_verify_gl_specialization_constants(
+      (uint32_t *)&spirv_data->SpirVModule->Binary[0],
+      spirv_data->SpirVModule->Length / 4,
+      spec_entries, numSpecializationConstants,
+      sh->Stage, pEntryPoint);
 
-   /* See previous spec comment */
-   if (!has_entry_point) {
+   switch (r) {
+   case SPIRV_VERIFY_OK:
+      break;
+   case SPIRV_VERIFY_PARSER_ERROR:
       _mesa_error(ctx, GL_INVALID_VALUE,
-                  "glSpecializeShaderARB(\"%s\" is not a valid entry point"
+                  "glSpecializeShaderARB(failed to parse entry point \"%s\""
                   " for shader)", pEntryPoint);
       goto end;
-   }
-
-   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
-      if (spec_entries[i].defined_on_module == false) {
-         _mesa_error(ctx, GL_INVALID_VALUE,
-                     "glSpecializeShaderARB(constant \"%i\" does not exist "
-                     "in shader)", spec_entries[i].id);
-         goto end;
+   case SPIRV_VERIFY_ENTRY_POINT_NOT_FOUND:
+      _mesa_error(ctx, GL_INVALID_VALUE,
+                  "glSpecializeShaderARB(could not find entry point \"%s\""
+                  " for shader)", pEntryPoint);
+      goto end;
+   case SPIRV_VERIFY_UNKNOWN_SPEC_INDEX:
+      for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+         if (spec_entries[i].defined_on_module == false) {
+            _mesa_error(ctx, GL_INVALID_VALUE,
+                        "glSpecializeShaderARB(constant \"%i\" does not exist "
+                        "in shader)", spec_entries[i].id);
+            break;
+         }
       }
+      goto end;
    }
 
    spirv_data->SpirVEntryPoint = ralloc_strdup(spirv_data, pEntryPoint);