Re: [Mesa-dev] [PATCH v3] glsl: enforce invariant conditions for built-in variables

2016-05-09 Thread Lars Hamre 
Looks like it, I didn't realized VARYING_SLOT_POS covered both cases.

On Mon, May 9, 2016 at 5:17 PM, Ilia Mirkin  wrote:
> On Mon, May 9, 2016 at 5:10 PM, Lars Hamre  wrote:
>> v3:
>>  - compare varying slot locations rather than names (Ilia Mirkin)
>> v2:
>>  - ES version check (Tapani Pälli)
>>
>> The conditions for which certain built-in special variables
>> can be declared invariant were not being checked.
>>
>> GLSL ES 1.00 specification, Section "Invariance and linkage" says:
>>
>> For the built-in special variables, gl_FragCoord can
>> only be declared invariant if and only if gl_Position is
>> declared invariant. Similarly gl_PointCoord can only be
>> declared invariant if and only if gl_PointSize is declared
>> invariant. It is an error to declare gl_FrontFacing as invariant.
>>
>> This fixes the following piglit tests in spec/glsl-es-1.00/linker:
>> glsl-fcoord-invariant
>> glsl-fface-invariant
>> glsl-pcoord-invariant
>>
>> Signed-off-by: Lars Hamre 
>>
>> ---
>>
>> CC: Ilia Mirkin 
>>
>> NOTE: Someone with access will need to commit this after the
>>   review process
>>
>>  src/compiler/glsl/link_varyings.cpp | 43 
>> +++--
>>  1 file changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/link_varyings.cpp 
>> b/src/compiler/glsl/link_varyings.cpp
>> index 34e82c7..155a4d9 100644
>> --- a/src/compiler/glsl/link_varyings.cpp
>> +++ b/src/compiler/glsl/link_varyings.cpp
>> @@ -352,13 +352,23 @@ cross_validate_outputs_to_inputs(struct 
>> gl_shader_program *prog,
>> glsl_symbol_table parameters;
>> ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
>>
>> +   bool is_gl_position_invariant = false;
>> +   bool is_gl_point_size_invariant = false;
>> +
>> /* Find all shader outputs in the "producer" stage.
>>  */
>> foreach_in_list(ir_instruction, node, producer->ir) {
>>ir_variable *const var = node->as_variable();
>>
>>if ((var == NULL) || (var->data.mode != ir_var_shader_out))
>> -continue;
>> + continue;
>> +
>> +  if (prog->IsES && prog->Version < 300) {
>> + if (var->data.location == VARYING_SLOT_POS)
>> +is_gl_position_invariant = var->data.invariant;
>> + if (var->data.location == VARYING_SLOT_PSIZ)
>> +is_gl_point_size_invariant = var->data.invariant;
>> +  }
>>
>>if (!var->data.explicit_location
>>|| var->data.location < VARYING_SLOT_VAR0)
>> @@ -442,7 +452,36 @@ cross_validate_outputs_to_inputs(struct 
>> gl_shader_program *prog,
>>ir_variable *const input = node->as_variable();
>>
>>if ((input == NULL) || (input->data.mode != ir_var_shader_in))
>> -continue;
>> + continue;
>> +
>> +  /*
>> +   * GLSL ES 1.00 specification, Section "Invariance and linkage" says:
>> +   *
>> +   *  "For the built-in special variables, gl_FragCoord can
>> +   *  only be declared invariant if and only if gl_Position is
>> +   *  declared invariant. Similarly gl_PointCoord can only be
>> +   *  declared invariant if and only if gl_PointSize is declared
>> +   *  invariant. It is an error to declare gl_FrontFacing as invariant."
>> +   */
>> +  if (prog->IsES && prog->Version < 300 && input->data.invariant) {
>> + if (input->data.location == VARYING_SLOT_FACE) {
>> +linker_error(prog,
>> + "gl_FrontFacing cannot be declared invariant");
>> +return;
>> + } else if (!is_gl_position_invariant &&
>> +!strcmp(input->name, "gl_FragCoord")) {
>
> I think you want input->data.location == VARYING_SLOT_POS here.
>
>> +linker_error(prog,
>> + "gl_FragCoord cannot be declared invariant "
>> + "unless gl_Position is also invariant");
>> +return;
>> + } else if (!is_gl_point_size_invariant &&
>> +input->data.location == VARYING_SLOT_PNTC) {
>> +linker_error(prog,
>> + "gl_PointCoord cannot be declared invariant "
>> + "unless gl_PointSize is also invariant");
>> +return;
>> + }
>> +  }
>>
>>if (strcmp(input->name, "gl_Color") == 0 && input->data.used) {
>>   const ir_variable *const front_color =
>> --
>> 2.5.5
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] glsl: enforce invariant conditions for built-in variables

2016-05-09 Thread Ilia Mirkin 
On Mon, May 9, 2016 at 5:10 PM, Lars Hamre  wrote:
> v3:
>  - compare varying slot locations rather than names (Ilia Mirkin)
> v2:
>  - ES version check (Tapani Pälli)
>
> The conditions for which certain built-in special variables
> can be declared invariant were not being checked.
>
> GLSL ES 1.00 specification, Section "Invariance and linkage" says:
>
> For the built-in special variables, gl_FragCoord can
> only be declared invariant if and only if gl_Position is
> declared invariant. Similarly gl_PointCoord can only be
> declared invariant if and only if gl_PointSize is declared
> invariant. It is an error to declare gl_FrontFacing as invariant.
>
> This fixes the following piglit tests in spec/glsl-es-1.00/linker:
> glsl-fcoord-invariant
> glsl-fface-invariant
> glsl-pcoord-invariant
>
> Signed-off-by: Lars Hamre 
>
> ---
>
> CC: Ilia Mirkin 
>
> NOTE: Someone with access will need to commit this after the
>   review process
>
>  src/compiler/glsl/link_varyings.cpp | 43 
> +++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index 34e82c7..155a4d9 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -352,13 +352,23 @@ cross_validate_outputs_to_inputs(struct 
> gl_shader_program *prog,
> glsl_symbol_table parameters;
> ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
>
> +   bool is_gl_position_invariant = false;
> +   bool is_gl_point_size_invariant = false;
> +
> /* Find all shader outputs in the "producer" stage.
>  */
> foreach_in_list(ir_instruction, node, producer->ir) {
>ir_variable *const var = node->as_variable();
>
>if ((var == NULL) || (var->data.mode != ir_var_shader_out))
> -continue;
> + continue;
> +
> +  if (prog->IsES && prog->Version < 300) {
> + if (var->data.location == VARYING_SLOT_POS)
> +is_gl_position_invariant = var->data.invariant;
> + if (var->data.location == VARYING_SLOT_PSIZ)
> +is_gl_point_size_invariant = var->data.invariant;
> +  }
>
>if (!var->data.explicit_location
>|| var->data.location < VARYING_SLOT_VAR0)
> @@ -442,7 +452,36 @@ cross_validate_outputs_to_inputs(struct 
> gl_shader_program *prog,
>ir_variable *const input = node->as_variable();
>
>if ((input == NULL) || (input->data.mode != ir_var_shader_in))
> -continue;
> + continue;
> +
> +  /*
> +   * GLSL ES 1.00 specification, Section "Invariance and linkage" says:
> +   *
> +   *  "For the built-in special variables, gl_FragCoord can
> +   *  only be declared invariant if and only if gl_Position is
> +   *  declared invariant. Similarly gl_PointCoord can only be
> +   *  declared invariant if and only if gl_PointSize is declared
> +   *  invariant. It is an error to declare gl_FrontFacing as invariant."
> +   */
> +  if (prog->IsES && prog->Version < 300 && input->data.invariant) {
> + if (input->data.location == VARYING_SLOT_FACE) {
> +linker_error(prog,
> + "gl_FrontFacing cannot be declared invariant");
> +return;
> + } else if (!is_gl_position_invariant &&
> +!strcmp(input->name, "gl_FragCoord")) {

I think you want input->data.location == VARYING_SLOT_POS here.

> +linker_error(prog,
> + "gl_FragCoord cannot be declared invariant "
> + "unless gl_Position is also invariant");
> +return;
> + } else if (!is_gl_point_size_invariant &&
> +input->data.location == VARYING_SLOT_PNTC) {
> +linker_error(prog,
> + "gl_PointCoord cannot be declared invariant "
> + "unless gl_PointSize is also invariant");
> +return;
> + }
> +  }
>
>if (strcmp(input->name, "gl_Color") == 0 && input->data.used) {
>   const ir_variable *const front_color =
> --
> 2.5.5
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] glsl: enforce invariant conditions for built-in variables

2016-05-09 Thread Lars Hamre 
v3:
 - compare varying slot locations rather than names (Ilia Mirkin)
v2:
 - ES version check (Tapani Pälli)

The conditions for which certain built-in special variables
can be declared invariant were not being checked.

GLSL ES 1.00 specification, Section "Invariance and linkage" says:

For the built-in special variables, gl_FragCoord can
only be declared invariant if and only if gl_Position is
declared invariant. Similarly gl_PointCoord can only be
declared invariant if and only if gl_PointSize is declared
invariant. It is an error to declare gl_FrontFacing as invariant.

This fixes the following piglit tests in spec/glsl-es-1.00/linker:
glsl-fcoord-invariant
glsl-fface-invariant
glsl-pcoord-invariant

Signed-off-by: Lars Hamre 

---

CC: Ilia Mirkin 

NOTE: Someone with access will need to commit this after the
  review process

 src/compiler/glsl/link_varyings.cpp | 43 +++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 34e82c7..155a4d9 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -352,13 +352,23 @@ cross_validate_outputs_to_inputs(struct gl_shader_program 
*prog,
glsl_symbol_table parameters;
ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };

+   bool is_gl_position_invariant = false;
+   bool is_gl_point_size_invariant = false;
+
/* Find all shader outputs in the "producer" stage.
 */
foreach_in_list(ir_instruction, node, producer->ir) {
   ir_variable *const var = node->as_variable();

   if ((var == NULL) || (var->data.mode != ir_var_shader_out))
-continue;
+ continue;
+
+  if (prog->IsES && prog->Version < 300) {
+ if (var->data.location == VARYING_SLOT_POS)
+is_gl_position_invariant = var->data.invariant;
+ if (var->data.location == VARYING_SLOT_PSIZ)
+is_gl_point_size_invariant = var->data.invariant;
+  }

   if (!var->data.explicit_location
   || var->data.location < VARYING_SLOT_VAR0)
@@ -442,7 +452,36 @@ cross_validate_outputs_to_inputs(struct gl_shader_program 
*prog,
   ir_variable *const input = node->as_variable();

   if ((input == NULL) || (input->data.mode != ir_var_shader_in))
-continue;
+ continue;
+
+  /*
+   * GLSL ES 1.00 specification, Section "Invariance and linkage" says:
+   *
+   *  "For the built-in special variables, gl_FragCoord can
+   *  only be declared invariant if and only if gl_Position is
+   *  declared invariant. Similarly gl_PointCoord can only be
+   *  declared invariant if and only if gl_PointSize is declared
+   *  invariant. It is an error to declare gl_FrontFacing as invariant."
+   */
+  if (prog->IsES && prog->Version < 300 && input->data.invariant) {
+ if (input->data.location == VARYING_SLOT_FACE) {
+linker_error(prog,
+ "gl_FrontFacing cannot be declared invariant");
+return;
+ } else if (!is_gl_position_invariant &&
+!strcmp(input->name, "gl_FragCoord")) {
+linker_error(prog,
+ "gl_FragCoord cannot be declared invariant "
+ "unless gl_Position is also invariant");
+return;
+ } else if (!is_gl_point_size_invariant &&
+input->data.location == VARYING_SLOT_PNTC) {
+linker_error(prog,
+ "gl_PointCoord cannot be declared invariant "
+ "unless gl_PointSize is also invariant");
+return;
+ }
+  }

   if (strcmp(input->name, "gl_Color") == 0 && input->data.used) {
  const ir_variable *const front_color =
--
2.5.5

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