Re: [Mesa-dev] [PATCH 16/17] glsl: Add link time checks for GLSL precision qualifiers

2015-08-05 Thread Iago Toral 
On Thu, 2015-07-30 at 12:33 +0200, Iago Toral wrote:
 On Wed, 2015-07-29 at 15:21 -0700, Ian Romanick wrote:
  On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez wrote:
   From: Iago Toral Quiroga ito...@igalia.com
   
   Currently, we only consider precision qualifiers at compile-time. This 
   patch
   adds precision information to ir_variable so we can also do link time 
   checks.
   Specifically, from the GLSL ES3 spec, 4.5.3 Precision Qualifiers:
   
   The same uniform declared in different shaders that are linked together
must have the same precision qualification.
   
   Notice that this patch will check the above also for GLSL ES globals that 
   are
   not uniforms. This is not explicitly stated in the spec, but seems to be
   the only consistent choice since we can only have one definition of a 
   global
   all its declarations should be identical, including precision qualifiers.
  
  That's not right.  Global variables from different stages that are not
  inputs/outputs or uniforms are distinct... they don't even have to be
  the same type.  ES shaders only allow a single compliation unit per
  stage, so we don't have to worry about inter-stage globals.
 
 Ugh, sorry, the commit log does not make a good job at explaining the
 situation. This patch does not produce a linker error for globals that
 are not uniforms, I only meant to say that for globals *in interface
 blocks*, for which we are producing a linker error in the case of type
 mismatches, precision will also be considered to decide if the types
 mismatch.
 
 Sorry for being so imprecise in the description, I'll fix the commit
 log. I guess with this clarification there are no issues with this,
 right?

Timothy pointed out to me recently that the GLSL ES spec has this
mention:

Precision qualifiers for outputs in one shader matched to inputs in
another shader need not match when both shaders are linked into the same
program. When both shaders are in separate programs, mismatched 
precision qualifiers will result in a program interface mismatch that
will result in program pipeline validation failures, as described in
section 7.4.1 (“Shader Interface Matching”) of the OpenGL ES 3.1
Specification.

This makes things a bit more complicated I guess, in any case it makes
clear that this is not exactly what the ES spec expects, so I need to
rethink how to approach this.

   These checks don't affect desktop GLSL shaders because we ignore precision
   information in this case (all variables have precision 
   GLSL_PRECISION_NONE).
   
   Fixes the following 5 dEQP tests:
   dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_1
   dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_2
   dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_3
   dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_4
   dEQP-GLES3.functional.shaders.linkage.uniform.block.precision_mismatch
   ---
src/glsl/linker.cpp | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
   
   diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
   index 12b7780..fd68f43 100644
   --- a/src/glsl/linker.cpp
   +++ b/src/glsl/linker.cpp
   @@ -958,13 +958,22 @@ cross_validate_globals(struct gl_shader_program 
   *prog,
  if (var-type-is_record()  
   existing-type-is_record()
   existing-type-record_compare(var-type)) {
 existing-type = var-type;
   -  } else {
   +  } else if (strcmp(var-type-name, 
   existing-type-name)) {
 linker_error(prog, %s `%s' declared as type 
  `%s' and type `%s'\n,
  mode_string(var),
  var-name, var-type-name,
  existing-type-name);
 return;
   +  } else {
   + /* The global is declared with the same type name 
   but the type
   +  * declarations mismatch (e.g. the same struct type 
   name, but
   +  * the actual struct declarations mismatch).
   +  */
   + linker_error(prog, %s `%s' declared with 
   mismatching definitions 
   +  of type `%s'\n,
   +  mode_string(var), var-name, 
   var-type-name);
   + return;
  }
}
 }
   @@ -1121,6 +1130,29 @@ cross_validate_globals(struct gl_shader_program 
   *prog,
mode_string(var), var-name);
   return;
}
   +/* From the GLSL ES3 spec, 4.5.3 Precision qualifiers:
   + *
   + * The same uniform declared in different shaders that are 
   linked
   + *

Re: [Mesa-dev] [PATCH 16/17] glsl: Add link time checks for GLSL precision qualifiers

2015-07-30 Thread Iago Toral 
On Wed, 2015-07-29 at 15:21 -0700, Ian Romanick wrote:
 On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez wrote:
  From: Iago Toral Quiroga ito...@igalia.com
  
  Currently, we only consider precision qualifiers at compile-time. This patch
  adds precision information to ir_variable so we can also do link time 
  checks.
  Specifically, from the GLSL ES3 spec, 4.5.3 Precision Qualifiers:
  
  The same uniform declared in different shaders that are linked together
   must have the same precision qualification.
  
  Notice that this patch will check the above also for GLSL ES globals that 
  are
  not uniforms. This is not explicitly stated in the spec, but seems to be
  the only consistent choice since we can only have one definition of a global
  all its declarations should be identical, including precision qualifiers.
 
 That's not right.  Global variables from different stages that are not
 inputs/outputs or uniforms are distinct... they don't even have to be
 the same type.  ES shaders only allow a single compliation unit per
 stage, so we don't have to worry about inter-stage globals.

Ugh, sorry, the commit log does not make a good job at explaining the
situation. This patch does not produce a linker error for globals that
are not uniforms, I only meant to say that for globals *in interface
blocks*, for which we are producing a linker error in the case of type
mismatches, precision will also be considered to decide if the types
mismatch.

Sorry for being so imprecise in the description, I'll fix the commit
log. I guess with this clarification there are no issues with this,
right?

Iago

  These checks don't affect desktop GLSL shaders because we ignore precision
  information in this case (all variables have precision GLSL_PRECISION_NONE).
  
  Fixes the following 5 dEQP tests:
  dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_1
  dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_2
  dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_3
  dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_4
  dEQP-GLES3.functional.shaders.linkage.uniform.block.precision_mismatch
  ---
   src/glsl/linker.cpp | 34 +-
   1 file changed, 33 insertions(+), 1 deletion(-)
  
  diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
  index 12b7780..fd68f43 100644
  --- a/src/glsl/linker.cpp
  +++ b/src/glsl/linker.cpp
  @@ -958,13 +958,22 @@ cross_validate_globals(struct gl_shader_program *prog,
 if (var-type-is_record()  existing-type-is_record()
  existing-type-record_compare(var-type)) {
existing-type = var-type;
  -  } else {
  +  } else if (strcmp(var-type-name, 
  existing-type-name)) {
linker_error(prog, %s `%s' declared as type 
 `%s' and type `%s'\n,
 mode_string(var),
 var-name, var-type-name,
 existing-type-name);
return;
  +  } else {
  + /* The global is declared with the same type name but 
  the type
  +  * declarations mismatch (e.g. the same struct type 
  name, but
  +  * the actual struct declarations mismatch).
  +  */
  + linker_error(prog, %s `%s' declared with mismatching 
  definitions 
  +  of type `%s'\n,
  +  mode_string(var), var-name, 
  var-type-name);
  + return;
 }
 }
  }
  @@ -1121,6 +1130,29 @@ cross_validate_globals(struct gl_shader_program 
  *prog,
   mode_string(var), var-name);
  return;
   }
  +/* From the GLSL ES3 spec, 4.5.3 Precision qualifiers:
  + *
  + * The same uniform declared in different shaders that are 
  linked
  + *  together must have the same precision qualification.
  + *
  + * In the GLSL ES2 spec this was resolved in the issue 
  amendments
  + * (10.3 Precision Qualifiers). The GLSL ES1 spec overlooked 
  this,
  + * but seems like an obvious error since we can only have one
  + * consistent definition of a global.
  + *
  + * The desktop GLSL spec does not include this reference
  + * because precision qualifiers are ignored. We will never
  + * hit this scenario in desktop GLSL though because we always 
  set
  + * the precision of variables to GLSL_PRECISION_NONE.
  + */
  +if (var-data.mode == ir_var_uniform) {
  +   if (existing-data.precision != var-data.precision)

Re: [Mesa-dev] [PATCH 16/17] glsl: Add link time checks for GLSL precision qualifiers

2015-07-29 Thread Ian Romanick 
On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez wrote:
 From: Iago Toral Quiroga ito...@igalia.com
 
 Currently, we only consider precision qualifiers at compile-time. This patch
 adds precision information to ir_variable so we can also do link time checks.
 Specifically, from the GLSL ES3 spec, 4.5.3 Precision Qualifiers:
 
 The same uniform declared in different shaders that are linked together
  must have the same precision qualification.
 
 Notice that this patch will check the above also for GLSL ES globals that are
 not uniforms. This is not explicitly stated in the spec, but seems to be
 the only consistent choice since we can only have one definition of a global
 all its declarations should be identical, including precision qualifiers.

That's not right.  Global variables from different stages that are not
inputs/outputs or uniforms are distinct... they don't even have to be
the same type.  ES shaders only allow a single compliation unit per
stage, so we don't have to worry about inter-stage globals.

 These checks don't affect desktop GLSL shaders because we ignore precision
 information in this case (all variables have precision GLSL_PRECISION_NONE).
 
 Fixes the following 5 dEQP tests:
 dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_1
 dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_2
 dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_3
 dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_4
 dEQP-GLES3.functional.shaders.linkage.uniform.block.precision_mismatch
 ---
  src/glsl/linker.cpp | 34 +-
  1 file changed, 33 insertions(+), 1 deletion(-)
 
 diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
 index 12b7780..fd68f43 100644
 --- a/src/glsl/linker.cpp
 +++ b/src/glsl/linker.cpp
 @@ -958,13 +958,22 @@ cross_validate_globals(struct gl_shader_program *prog,
if (var-type-is_record()  existing-type-is_record()
 existing-type-record_compare(var-type)) {
   existing-type = var-type;
 -  } else {
 +  } else if (strcmp(var-type-name, existing-type-name)) {
   linker_error(prog, %s `%s' declared as type 
`%s' and type `%s'\n,
mode_string(var),
var-name, var-type-name,
existing-type-name);
   return;
 +  } else {
 + /* The global is declared with the same type name but 
 the type
 +  * declarations mismatch (e.g. the same struct type 
 name, but
 +  * the actual struct declarations mismatch).
 +  */
 + linker_error(prog, %s `%s' declared with mismatching 
 definitions 
 +  of type `%s'\n,
 +  mode_string(var), var-name, 
 var-type-name);
 + return;
}
  }
   }
 @@ -1121,6 +1130,29 @@ cross_validate_globals(struct gl_shader_program *prog,
  mode_string(var), var-name);
 return;
  }
 +/* From the GLSL ES3 spec, 4.5.3 Precision qualifiers:
 + *
 + * The same uniform declared in different shaders that are 
 linked
 + *  together must have the same precision qualification.
 + *
 + * In the GLSL ES2 spec this was resolved in the issue amendments
 + * (10.3 Precision Qualifiers). The GLSL ES1 spec overlooked 
 this,
 + * but seems like an obvious error since we can only have one
 + * consistent definition of a global.
 + *
 + * The desktop GLSL spec does not include this reference
 + * because precision qualifiers are ignored. We will never
 + * hit this scenario in desktop GLSL though because we always set
 + * the precision of variables to GLSL_PRECISION_NONE.
 + */
 +if (var-data.mode == ir_var_uniform) {
 +   if (existing-data.precision != var-data.precision) {
 +  linker_error(prog, declarations for %s `%s` have 
 +   mismatching precision qualifiers\n,
 +   mode_string(var), var-name);
 +  return;
 +   }
 +}
} else
   variables.add_variable(var);
}
 

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

[Mesa-dev] [PATCH 16/17] glsl: Add link time checks for GLSL precision qualifiers

2015-07-29 Thread Samuel Iglesias Gonsalvez 
From: Iago Toral Quiroga ito...@igalia.com

Currently, we only consider precision qualifiers at compile-time. This patch
adds precision information to ir_variable so we can also do link time checks.
Specifically, from the GLSL ES3 spec, 4.5.3 Precision Qualifiers:

The same uniform declared in different shaders that are linked together
 must have the same precision qualification.

Notice that this patch will check the above also for GLSL ES globals that are
not uniforms. This is not explicitly stated in the spec, but seems to be
the only consistent choice since we can only have one definition of a global
all its declarations should be identical, including precision qualifiers.

These checks don't affect desktop GLSL shaders because we ignore precision
information in this case (all variables have precision GLSL_PRECISION_NONE).

Fixes the following 5 dEQP tests:
dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_1
dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_2
dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_3
dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_4
dEQP-GLES3.functional.shaders.linkage.uniform.block.precision_mismatch
---
 src/glsl/linker.cpp | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 12b7780..fd68f43 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -958,13 +958,22 @@ cross_validate_globals(struct gl_shader_program *prog,
   if (var-type-is_record()  existing-type-is_record()
existing-type-record_compare(var-type)) {
  existing-type = var-type;
-  } else {
+  } else if (strcmp(var-type-name, existing-type-name)) {
  linker_error(prog, %s `%s' declared as type 
   `%s' and type `%s'\n,
   mode_string(var),
   var-name, var-type-name,
   existing-type-name);
  return;
+  } else {
+ /* The global is declared with the same type name but the 
type
+  * declarations mismatch (e.g. the same struct type name, 
but
+  * the actual struct declarations mismatch).
+  */
+ linker_error(prog, %s `%s' declared with mismatching 
definitions 
+  of type `%s'\n,
+  mode_string(var), var-name, 
var-type-name);
+ return;
   }
   }
}
@@ -1121,6 +1130,29 @@ cross_validate_globals(struct gl_shader_program *prog,
 mode_string(var), var-name);
return;
 }
+/* From the GLSL ES3 spec, 4.5.3 Precision qualifiers:
+ *
+ * The same uniform declared in different shaders that are linked
+ *  together must have the same precision qualification.
+ *
+ * In the GLSL ES2 spec this was resolved in the issue amendments
+ * (10.3 Precision Qualifiers). The GLSL ES1 spec overlooked this,
+ * but seems like an obvious error since we can only have one
+ * consistent definition of a global.
+ *
+ * The desktop GLSL spec does not include this reference
+ * because precision qualifiers are ignored. We will never
+ * hit this scenario in desktop GLSL though because we always set
+ * the precision of variables to GLSL_PRECISION_NONE.
+ */
+if (var-data.mode == ir_var_uniform) {
+   if (existing-data.precision != var-data.precision) {
+  linker_error(prog, declarations for %s `%s` have 
+   mismatching precision qualifiers\n,
+   mode_string(var), var-name);
+  return;
+   }
+}
 } else
variables.add_variable(var);
   }
-- 
2.4.3

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