[Mesa-dev] [PATCH v2 24/25] i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders

[Timothy Arceri]

On 01/12/17 18:23, Eduardo Lima Mitev wrote:
> On 12/01/2017 04:44 AM, Timothy Arceri wrote:
>>
>>
>> On 01/12/17 04:28, Eduardo Lima Mitev wrote:
>>> This is the main fork of the shader compilation code-path, where a NIR
>>> shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
>>> depending on its nature..
>>>
>>> v2: Use 'spirv_data' member from gl_linked_shader to know which method
>>>      to call. (Timothy Arceri)
>>> ---
>>>    src/mesa/drivers/dri/i965/brw_program.c | 14 ++++++++++++--
>>>    1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_program.c
>>> b/src/mesa/drivers/dri/i965/brw_program.c
>>> index 755d4973cc0..596118f2fe5 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_program.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_program.c
>>> @@ -31,6 +31,7 @@
>>>      #include <pthread.h>
>>>    #include "main/imports.h"
>>> +#include "main/glspirv.h"
>>>    #include "program/prog_parameter.h"
>>>    #include "program/prog_print.h"
>>>    #include "program/prog_to_nir.h"
>>> @@ -73,9 +74,18 @@ brw_create_nir(struct brw_context *brw,
>>>          ctx->Const.ShaderCompilerOptions[stage].NirOptions;
>>>       nir_shader *nir;
>>>    -   /* First, lower the GLSL IR or Mesa IR to NIR */
>>> +   /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
>>>       if (shader_prog) {
>>> -      nir = glsl_to_nir(shader_prog, stage, options);
>>> +      bool is_spirv_shader =
>>> +         (shader_prog->_LinkedShaders[stage]->spirv_data != NULL);
>>> +
>>> +      if (!is_spirv_shader) {
>>> +         nir = glsl_to_nir(shader_prog, stage, options);
>>> +      } else {
>>> +         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
>>> +      }
>>> +      assert (nir);
>>
>> Rather than messing around with bools, null checks and !'s I'd just
>> change this to:
>>
>>        /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
>>        if (shader_prog->_LinkedShaders[stage]->spirv_data) {
>>           nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
>>        } else {
>>           nir = glsl_to_nir(shader_prog, stage, options);
>>        }
>>        assert (nir);
>>
>>
> 
> My intention was to make it clearer that we are using a pointer nullness
> to decide code-path.
> I don't care much so I will use your inlined version above, maybe just
> adding a comment.

I don't think you even need a comment. The code speaks for itself :)


> 
> Thanks,
> 
> Eduardo
> 
>>> +
>>>          nir_remove_dead_variables(nir, nir_var_shader_in |
>>> nir_var_shader_out);
>>>          nir_lower_returns(nir);
>>>          nir_validate_shader(nir);
>>>
>>
>