[Mesa-dev] [PATCH 16/24] mesa: Implement glSpecializeShaderARB

[Alejandro Piñeiro]

On 27/11/17 03:22, Timothy Arceri wrote:
> I suspect this patch doesn't compile. I think pEntryPoint is meant to
> be passed as an argument to this function.

It is the second parameter of the method. But it is true that it doesn't
appear on this patch. Seems that the diff skipped some lines there.

In any case, note that the method was added on patch "mesa: add
GL_ARB_gl_spirv boilerplate", with pEntryPoint as second parameter, and
no other patch is changing that.

>
> On 16/11/17 00:22, Eduardo Lima Mitev wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> v2: use gl_spirv_validation instead of spirv_to_nir.
>>     This method just validates the shader. The conversion to NIR will
>>     happen later, during linking. (Alejandro Piñeiro)
>>
>> v3: Use gl_shader_spirv_data struct to store the SPIR-V data.
>>     (Eduardo Lima)
>> ---
>>   src/mesa/main/glspirv.c | 107
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>>
>> diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
>> index 390fa9e6984..fe954b05c7b 100644
>> --- a/src/mesa/main/glspirv.c
>> +++ b/src/mesa/main/glspirv.c
>> @@ -24,6 +24,10 @@
>>   #include "glspirv.h"
>>     #include "errors.h"
>> +#include "shaderobj.h"
>> +
>> +#include "compiler/nir/nir.h"
>> +#include "compiler/spirv/nir_spirv.h"
>>     #include "util/u_atomic.h"
>>   @@ -120,4 +124,107 @@ _mesa_SpecializeShaderARB(GLuint shader,
>>                             const GLuint *pConstantIndex,
>>                             const GLuint *pConstantValue)
>>   {
>> +   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) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION, "glSpecializeShaderARB");
>> +      return;
>> +   }
>> +
>> +   sh = _mesa_lookup_shader_err(ctx, shader, "glSpecializeShaderARB");
>> +   if (!sh)
>> +      return;
>> +
>> +   if (!sh->SpirVBinary) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                  "glSpecializeShaderARB(not SPIR-V)");
>> +      return;
>> +   }
>> +
>> +   if (sh->CompileStatus) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                  "glSpecializeShaderARB(already specialized)");
>> +      return;
>> +   }
>> +
>> +   struct gl_shader_spirv_data *spirv_data = sh->spirv_data;
>> +   assert(spirv_data);
>> +
>> +   /* From the GL_ARB_gl_spirv spec:
>> +    *
>> +    *    "The OpenGL API expects the SPIR-V module to have already been
>> +    *     validated, and can return an error if it discovers
>> anything invalid
>> +    *     in the module. An invalid SPIR-V module is allowed to
>> result in
>> +    *     undefined behavior."
>> +    *
>> +    * However, the following errors still need to be detected (from
>> the same
>> +    * spec):
>> +    *
>> +    *    "INVALID_VALUE is generated if <pEntryPoint> does not name
>> a valid
>> +    *     entry point for <shader>.
>> +    *
>> +    *     INVALID_VALUE is generated if any element of <pConstantIndex>
>> +    *     refers to a specialization constant that does not exist in
>> the
>> +    *     shader module contained in <shader>."
>> +    *
>> +    * We cannot flag those errors a-priori because detecting them
>> requires
>> +    * parsing the module. However, flagging them during
>> specialization is okay,
>> +    * since it makes no difference in terms of application-visible
>> state.
>> +    */
>> +   spec_entries = calloc(sizeof(*spec_entries),
>> numSpecializationConstants);
>> +
>> +   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
>> +      spec_entries[i].id = pConstantIndex[i];
>> +      spec_entries[i].data32 = pConstantValue[i];
>> +      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);
>> +
>> +   /* See previous spec comment */
>> +   if (!has_entry_point) {
>> +      _mesa_error(ctx, GL_INVALID_VALUE,
>> +                  "glSpecializeShaderARB(\"%s\" is not a valid entry
>> point"
>> +                  " 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;
>> +      }
>> +   }
>> +
>> +   spirv_data->SpirVEntryPoint = ralloc_strdup(spirv_data,
>> pEntryPoint);
>> +
>> +   /* Note that we didn't make a real compilation of the module
>> (spirv_to_nir),
>> +    * but just checked some error conditions. Real "compilation"
>> will be done
>> +    * later, upon linking.
>> +    */
>> +   sh->CompileStatus = compile_success;
>> +
>> +   spirv_data->NumSpecializationConstants = numSpecializationConstants;
>> +   spirv_data->SpecializationConstantsIndex =
>> +      rzalloc_array_size(spirv_data, sizeof(GLuint),
>> +                         numSpecializationConstants);
>> +   spirv_data->SpecializationConstantsValue =
>> +      rzalloc_array_size(spirv_data, sizeof(GLuint),
>> +                         numSpecializationConstants);
>> +   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
>> +      spirv_data->SpecializationConstantsIndex[i] = pConstantIndex[i];
>> +      spirv_data->SpecializationConstantsValue[i] = pConstantValue[i];
>> +   }
>> +
>> + end:
>> +   free(spec_entries);
>>   }
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev