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

2017-12-03 Thread 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 
   #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);






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

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

2017-11-30 Thread Eduardo Lima Mitev 
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 
>>   #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.

Thanks,

Eduardo

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

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

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

2017-11-30 Thread Timothy Arceri 



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 

  #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);



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


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

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

2017-11-30 Thread Eduardo Lima Mitev 
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 
 #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);
+
   nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
   nir_lower_returns(nir);
   nir_validate_shader(nir);
-- 
2.15.0

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