Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-17 Thread Ilia Mirkin 
On Mon, Nov 16, 2015 at 11:44 AM, Ilia Mirkin  wrote:
> On Mon, Nov 16, 2015 at 11:42 AM, Samuel Iglesias Gonsálvez
>  wrote:
>>
>>
>> On 16/11/15 17:34, Ilia Mirkin wrote:
>>> On Mon, Nov 16, 2015 at 11:29 AM, Samuel Iglesias Gonsálvez
>>>  wrote:


 On 16/11/15 13:07, Tapani Pälli wrote:
>
> On 11/16/2015 01:35 PM, Tapani Pälli wrote:
>>
>>
>> On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
>>> Hello Ilia, Tapani:
>>>
>>> I have reproduced the issue with a piglit test but not with the trace
>>> uploaded in the bug report :-(
>>>
>>> The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks
>>>
>>> I have upload a branch with some fixes at Igalia's mesa repo:
>>>
>>> Git repo: https://github.com/Igalia/mesa.git
>>> Branch: wip/siglesias/precision-fixes
>>>
>>> But as this error might come from other initializations that I might
>>> overlook:
>>> * Ilia: Could you test if this issue is still happening to you? As I
>>> cannot reproduce it locally, I might be forgetting something.
>>> * Tapani: Could you do a quick run on CTS to check I have not broken
>>> anything?
>>
>> Sure thing, I'll run testing. FWIW one of the patches was identical to
>> my fix sent for fixing tessellation shader problems:
>>
>> http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html
>
> No CTS regressions with these patches, I've gone through these and
> changes look good to me!
>
>

 OK, once Ilia replies that the issue is fixed with those patches, I will
 send them for review to the mailing list :-)
>>>
>>> I won't have time to look until tonight. However the repro steps were
>>> pretty simple... download the trace and run through valgrind. Probably
>>> tons of other ways to trigger it too, of course... I'd esp look for
>>> piglits that have uniform structs.
>>>
>>
>> The problem is that I could not reproduce it with the trace. That's why
>> I am asking.
>>
>> I reproduce it with a piglit tests, but maybe precision is uninitialized
>> in other cases. Tomorrow I will do some more testing, just in case.
>
> Well, irrespective of other cases, if the things you're fixing are
> real fixes, no need to wait on me. I'll be sure to complain again if I
> still see problems. FWIW I did see them with nouveau, not i965. I
> suspect llvmpipe would take the same paths.

With all the latest patches that have landed, valgrind is happy again.
[At least in regards to this... it's got complaints about other
things, but those aren't your fault.]

Thanks for fixing!

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Samuel Iglesias Gonsálvez 


On 16/11/15 17:34, Ilia Mirkin wrote:
> On Mon, Nov 16, 2015 at 11:29 AM, Samuel Iglesias Gonsálvez
>  wrote:
>>
>>
>> On 16/11/15 13:07, Tapani Pälli wrote:
>>>
>>> On 11/16/2015 01:35 PM, Tapani Pälli wrote:


 On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
> Hello Ilia, Tapani:
>
> I have reproduced the issue with a piglit test but not with the trace
> uploaded in the bug report :-(
>
> The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks
>
> I have upload a branch with some fixes at Igalia's mesa repo:
>
> Git repo: https://github.com/Igalia/mesa.git
> Branch: wip/siglesias/precision-fixes
>
> But as this error might come from other initializations that I might
> overlook:
> * Ilia: Could you test if this issue is still happening to you? As I
> cannot reproduce it locally, I might be forgetting something.
> * Tapani: Could you do a quick run on CTS to check I have not broken
> anything?

 Sure thing, I'll run testing. FWIW one of the patches was identical to
 my fix sent for fixing tessellation shader problems:

 http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html
>>>
>>> No CTS regressions with these patches, I've gone through these and
>>> changes look good to me!
>>>
>>>
>>
>> OK, once Ilia replies that the issue is fixed with those patches, I will
>> send them for review to the mailing list :-)
> 
> I won't have time to look until tonight. However the repro steps were
> pretty simple... download the trace and run through valgrind. Probably
> tons of other ways to trigger it too, of course... I'd esp look for
> piglits that have uniform structs.
>

The problem is that I could not reproduce it with the trace. That's why
I am asking.

I reproduce it with a piglit tests, but maybe precision is uninitialized
in other cases. Tomorrow I will do some more testing, just in case.

Thanks,

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Tapani Pälli 


On 11/16/2015 01:35 PM, Tapani Pälli wrote:



On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:

Hello Ilia, Tapani:

I have reproduced the issue with a piglit test but not with the trace
uploaded in the bug report :-(

The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks

I have upload a branch with some fixes at Igalia's mesa repo:

Git repo: https://github.com/Igalia/mesa.git
Branch: wip/siglesias/precision-fixes

But as this error might come from other initializations that I might
overlook:
* Ilia: Could you test if this issue is still happening to you? As I
cannot reproduce it locally, I might be forgetting something.
* Tapani: Could you do a quick run on CTS to check I have not broken
anything?


Sure thing, I'll run testing. FWIW one of the patches was identical to
my fix sent for fixing tessellation shader problems:

http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html


No CTS regressions with these patches, I've gone through these and 
changes look good to me!




Thanks!

Sam


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

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Ilia Mirkin 
On Mon, Nov 16, 2015 at 11:29 AM, Samuel Iglesias Gonsálvez
 wrote:
>
>
> On 16/11/15 13:07, Tapani Pälli wrote:
>>
>> On 11/16/2015 01:35 PM, Tapani Pälli wrote:
>>>
>>>
>>> On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
 Hello Ilia, Tapani:

 I have reproduced the issue with a piglit test but not with the trace
 uploaded in the bug report :-(

 The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks

 I have upload a branch with some fixes at Igalia's mesa repo:

 Git repo: https://github.com/Igalia/mesa.git
 Branch: wip/siglesias/precision-fixes

 But as this error might come from other initializations that I might
 overlook:
 * Ilia: Could you test if this issue is still happening to you? As I
 cannot reproduce it locally, I might be forgetting something.
 * Tapani: Could you do a quick run on CTS to check I have not broken
 anything?
>>>
>>> Sure thing, I'll run testing. FWIW one of the patches was identical to
>>> my fix sent for fixing tessellation shader problems:
>>>
>>> http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html
>>
>> No CTS regressions with these patches, I've gone through these and
>> changes look good to me!
>>
>>
>
> OK, once Ilia replies that the issue is fixed with those patches, I will
> send them for review to the mailing list :-)

I won't have time to look until tonight. However the repro steps were
pretty simple... download the trace and run through valgrind. Probably
tons of other ways to trigger it too, of course... I'd esp look for
piglits that have uniform structs.

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Samuel Iglesias Gonsálvez 


On 16/11/15 13:07, Tapani Pälli wrote:
> 
> On 11/16/2015 01:35 PM, Tapani Pälli wrote:
>>
>>
>> On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
>>> Hello Ilia, Tapani:
>>>
>>> I have reproduced the issue with a piglit test but not with the trace
>>> uploaded in the bug report :-(
>>>
>>> The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks
>>>
>>> I have upload a branch with some fixes at Igalia's mesa repo:
>>>
>>> Git repo: https://github.com/Igalia/mesa.git
>>> Branch: wip/siglesias/precision-fixes
>>>
>>> But as this error might come from other initializations that I might
>>> overlook:
>>> * Ilia: Could you test if this issue is still happening to you? As I
>>> cannot reproduce it locally, I might be forgetting something.
>>> * Tapani: Could you do a quick run on CTS to check I have not broken
>>> anything?
>>
>> Sure thing, I'll run testing. FWIW one of the patches was identical to
>> my fix sent for fixing tessellation shader problems:
>>
>> http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html
> 
> No CTS regressions with these patches, I've gone through these and
> changes look good to me!
> 
> 

OK, once Ilia replies that the issue is fixed with those patches, I will
send them for review to the mailing list :-)

Thanks!

Sam

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Ilia Mirkin 
On Mon, Nov 16, 2015 at 11:42 AM, Samuel Iglesias Gonsálvez
 wrote:
>
>
> On 16/11/15 17:34, Ilia Mirkin wrote:
>> On Mon, Nov 16, 2015 at 11:29 AM, Samuel Iglesias Gonsálvez
>>  wrote:
>>>
>>>
>>> On 16/11/15 13:07, Tapani Pälli wrote:

 On 11/16/2015 01:35 PM, Tapani Pälli wrote:
>
>
> On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
>> Hello Ilia, Tapani:
>>
>> I have reproduced the issue with a piglit test but not with the trace
>> uploaded in the bug report :-(
>>
>> The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks
>>
>> I have upload a branch with some fixes at Igalia's mesa repo:
>>
>> Git repo: https://github.com/Igalia/mesa.git
>> Branch: wip/siglesias/precision-fixes
>>
>> But as this error might come from other initializations that I might
>> overlook:
>> * Ilia: Could you test if this issue is still happening to you? As I
>> cannot reproduce it locally, I might be forgetting something.
>> * Tapani: Could you do a quick run on CTS to check I have not broken
>> anything?
>
> Sure thing, I'll run testing. FWIW one of the patches was identical to
> my fix sent for fixing tessellation shader problems:
>
> http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html

 No CTS regressions with these patches, I've gone through these and
 changes look good to me!


>>>
>>> OK, once Ilia replies that the issue is fixed with those patches, I will
>>> send them for review to the mailing list :-)
>>
>> I won't have time to look until tonight. However the repro steps were
>> pretty simple... download the trace and run through valgrind. Probably
>> tons of other ways to trigger it too, of course... I'd esp look for
>> piglits that have uniform structs.
>>
>
> The problem is that I could not reproduce it with the trace. That's why
> I am asking.
>
> I reproduce it with a piglit tests, but maybe precision is uninitialized
> in other cases. Tomorrow I will do some more testing, just in case.

Well, irrespective of other cases, if the things you're fixing are
real fixes, no need to wait on me. I'll be sure to complain again if I
still see problems. FWIW I did see them with nouveau, not i965. I
suspect llvmpipe would take the same paths.

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Samuel Iglesias Gonsálvez 


On 16/11/15 12:35, Tapani Pälli wrote:
> 
> 
> On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:
>> Hello Ilia, Tapani:
>>
>> I have reproduced the issue with a piglit test but not with the trace
>> uploaded in the bug report :-(
>>
>> The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks
>>
>> I have upload a branch with some fixes at Igalia's mesa repo:
>>
>> Git repo: https://github.com/Igalia/mesa.git
>> Branch: wip/siglesias/precision-fixes
>>
>> But as this error might come from other initializations that I might
>> overlook:
>> * Ilia: Could you test if this issue is still happening to you? As I
>> cannot reproduce it locally, I might be forgetting something.
>> * Tapani: Could you do a quick run on CTS to check I have not broken
>> anything?
> 
> Sure thing, I'll run testing. FWIW one of the patches was identical to
> my fix sent for fixing tessellation shader problems:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html
> 

OK, thanks. I have reviewed your patch.

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Tapani Pälli 



On 11/16/2015 01:29 PM, Samuel Iglesias Gonsálvez wrote:

Hello Ilia, Tapani:

I have reproduced the issue with a piglit test but not with the trace
uploaded in the bug report :-(

The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks

I have upload a branch with some fixes at Igalia's mesa repo:

Git repo: https://github.com/Igalia/mesa.git
Branch: wip/siglesias/precision-fixes

But as this error might come from other initializations that I might
overlook:
* Ilia: Could you test if this issue is still happening to you? As I
cannot reproduce it locally, I might be forgetting something.
* Tapani: Could you do a quick run on CTS to check I have not broken
anything?


Sure thing, I'll run testing. FWIW one of the patches was identical to 
my fix sent for fixing tessellation shader problems:


http://lists.freedesktop.org/archives/mesa-dev/2015-November/100396.html


Thanks!

Sam


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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Samuel Iglesias Gonsálvez 


On 13/11/15 21:38, Ilia Mirkin wrote:
> On Fri, Nov 13, 2015 at 2:37 PM, Ilia Mirkin  wrote:
>> Looks like valgrind hates this for some reason. I'm seeing lots of
>>
>> ==16821== Conditional jump or move depends on uninitialised value(s)
>> ==16821==at 0xA074D09: glsl_type::record_compare(glsl_type const*)
>> const (glsl_types.cpp:783)
>>
>> Where line 783 is:
>>
>>   if (this->fields.structure[i].precision
>>   != b->fields.structure[i].precision)
>>
>> This happens with the trace from
>> https://bugs.freedesktop.org/show_bug.cgi?id=92229 but I suspect it
>> happens with just about anything with structs.
> 
> I tried the following but no go. I'm giving up for now.
> 

OK, I can reproduce this valgrind error. I am going to debug it.

Thanks!

Sam

>   -ilia
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 51ea183..92f8b37 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -6584,6 +6584,8 @@ ast_interface_block::hir(exec_list *instructions,
> earlier_per_vertex->fields.structure[j].sample;
>  fields[i].patch =
> earlier_per_vertex->fields.structure[j].patch;
> +fields[i].precision =
> +   earlier_per_vertex->fields.structure[j].precision;
>   }
>}
> 
> diff --git a/src/glsl/nir/glsl_types.cpp b/src/glsl/nir/glsl_types.cpp
> index 975b815..7345765 100644
> --- a/src/glsl/nir/glsl_types.cpp
> +++ b/src/glsl/nir/glsl_types.cpp
> @@ -124,6 +124,7 @@ glsl_type::glsl_type(const glsl_struct_field
> *fields, unsigned num_fields,
>this->fields.structure[i].sample = fields[i].sample;
>this->fields.structure[i].matrix_layout = fields[i].matrix_layout;
>this->fields.structure[i].patch = fields[i].patch;
> +  this->fields.structure[i].precision = fields[i].precision;
>this->fields.structure[i].image_read_only = fields[i].image_read_only;
>this->fields.structure[i].image_write_only = 
> fields[i].image_write_only;
>this->fields.structure[i].image_coherent = fields[i].image_coherent;
> diff --git a/src/glsl/nir/glsl_types.h b/src/glsl/nir/glsl_types.h
> index d841a32..f3a0cf8 100644
> --- a/src/glsl/nir/glsl_types.h
> +++ b/src/glsl/nir/glsl_types.h
> @@ -851,7 +851,7 @@ struct glsl_struct_field {
> 
> glsl_struct_field(const struct glsl_type *_type, const char *_name)
>: type(_type), name(_name), location(-1), interpolation(0), 
> centroid(0),
> -sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED), patch(0)
> +sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED),
> patch(0), precision(0)
> {
>/* empty */
> }
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-16 Thread Samuel Iglesias Gonsálvez 
Hello Ilia, Tapani:

I have reproduced the issue with a piglit test but not with the trace
uploaded in the bug report :-(

The piglit test was: bin/arb_shader_storage_buffer_object-maxblocks

I have upload a branch with some fixes at Igalia's mesa repo:

Git repo: https://github.com/Igalia/mesa.git
Branch: wip/siglesias/precision-fixes

But as this error might come from other initializations that I might
overlook:
* Ilia: Could you test if this issue is still happening to you? As I
cannot reproduce it locally, I might be forgetting something.
* Tapani: Could you do a quick run on CTS to check I have not broken
anything?

Thanks!

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

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-13 Thread Ilia Mirkin 
Looks like valgrind hates this for some reason. I'm seeing lots of

==16821== Conditional jump or move depends on uninitialised value(s)
==16821==at 0xA074D09: glsl_type::record_compare(glsl_type const*)
const (glsl_types.cpp:783)

Where line 783 is:

  if (this->fields.structure[i].precision
  != b->fields.structure[i].precision)

This happens with the trace from
https://bugs.freedesktop.org/show_bug.cgi?id=92229 but I suspect it
happens with just about anything with structs.

  -ilia


On Wed, Nov 11, 2015 at 7:45 AM, Samuel Iglesias Gonsálvez
 wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez 
>
> On 06/11/15 13:03, Tapani Pälli wrote:
>> From: Iago Toral Quiroga 
>>
>> We will need this later on when we implement proper support for
>> precision qualifiers in the drivers and also to do link time checks for
>> uniforms as indicated by the spec.
>>
>> This patch also adds compile-time checks for variables without precision
>> information (currently, Mesa only checks that a default precision is set
>> for floats in fragment shaders).
>>
>> As indicated by Ian, the addition of the precision information to
>> ir_variable has been done using a bitfield and pahole to identify an
>> available hole so that memory requirements for ir_variable stay the
>> same.
>>
>> v2 (Ian):
>>   - Avoid if-ladders by defining arrays of supported sampler names and
>> indexing
>> into them with type->sampler_array + 2 * type->sampler_shadow
>>   - Make the code that selects the precision qualifier to use an utility
>> function
>>   - Fix a typo
>>
>> v3 (Tapani):
>>   - rebased
>>   - squashed in "Precision qualifiers are not allowed on structs"
>>   - fixed select_gles_precision for sampler arrays
>>   - fixed precision_qualifier_allowed for arrays of structs
>>
>> v4 (Tapani):
>>   - add atomic_uint handling
>>   - do not allow precision qualifier on images
>>   (issues reported by Marta)
>>
>> v5 (Tapani):
>>   - support precision qualifier on image types
>> ---
>>  src/glsl/ast_to_hir.cpp | 296 
>> 
>>  src/glsl/ir.h   |  13 ++
>>  src/glsl/nir/glsl_types.cpp |   4 +
>>  src/glsl/nir/glsl_types.h   |  11 ++
>>  4 files changed, 301 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index b6d662b..1240615 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
>>  * From this, we infer that GLSL 1.30 (and later) should allow precision
>>  * qualifiers on sampler types just like float and integer types.
>>  */
>> -   return type->is_float()
>> +   return (type->is_float()
>> || type->is_integer()
>> -   || type->is_record()
>> -   || type->contains_opaque();
>> +   || type->contains_opaque())
>> +   && !type->without_array()->is_record();
>>  }
>>
>>  const glsl_type *
>> @@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
>> return type;
>>  }
>>
>> -const glsl_type *
>> -ast_fully_specified_type::glsl_type(const char **name,
>> -struct _mesa_glsl_parse_state *state) 
>> const
>> +/**
>> + * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
>> + *
>> + * "The precision statement
>> + *
>> + *precision precision-qualifier type;
>> + *
>> + *  can be used to establish a default precision qualifier. The type field 
>> can
>> + *  be either int or float or any of the sampler types, (...) If type is 
>> float,
>> + *  the directive applies to non-precision-qualified floating point type
>> + *  (scalar, vector, and matrix) declarations. If type is int, the directive
>> + *  applies to all non-precision-qualified integer type (scalar, vector, 
>> signed,
>> + *  and unsigned) declarations."
>> + *
>> + * We use the symbol table to keep the values of the default precisions for
>> + * each 'type' in each scope and we use the 'type' string from the precision
>> + * statement as key in the symbol table. When we want to retrieve the 
>> default
>> + * precision associated with a given glsl_type we need to know the type 
>> string
>> + * associated with it. This is what this function returns.
>> + */
>> +static const char *
>> +get_type_name_for_precision_qualifier(const glsl_type *type)
>>  {
>> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
>> -
>> -   if (type == NULL)
>> -  return NULL;
>> +   switch (type->base_type) {
>> +   case GLSL_TYPE_FLOAT:
>> +  return "float";
>> +   case GLSL_TYPE_UINT:
>> +   case GLSL_TYPE_INT:
>> +  return "int";
>> +   case GLSL_TYPE_ATOMIC_UINT:
>> +  return "atomic_uint";
>> +   case GLSL_TYPE_IMAGE:
>> +   /* fallthrough */
>> +   case GLSL_TYPE_SAMPLER: {
>> +  const unsigned type_idx =
>> + type->sampler_array + 2 * type->sampler_shadow;

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-13 Thread Ilia Mirkin 
On Fri, Nov 13, 2015 at 2:37 PM, Ilia Mirkin  wrote:
> Looks like valgrind hates this for some reason. I'm seeing lots of
>
> ==16821== Conditional jump or move depends on uninitialised value(s)
> ==16821==at 0xA074D09: glsl_type::record_compare(glsl_type const*)
> const (glsl_types.cpp:783)
>
> Where line 783 is:
>
>   if (this->fields.structure[i].precision
>   != b->fields.structure[i].precision)
>
> This happens with the trace from
> https://bugs.freedesktop.org/show_bug.cgi?id=92229 but I suspect it
> happens with just about anything with structs.

I tried the following but no go. I'm giving up for now.

  -ilia

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 51ea183..92f8b37 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -6584,6 +6584,8 @@ ast_interface_block::hir(exec_list *instructions,
earlier_per_vertex->fields.structure[j].sample;
 fields[i].patch =
earlier_per_vertex->fields.structure[j].patch;
+fields[i].precision =
+   earlier_per_vertex->fields.structure[j].precision;
  }
   }

diff --git a/src/glsl/nir/glsl_types.cpp b/src/glsl/nir/glsl_types.cpp
index 975b815..7345765 100644
--- a/src/glsl/nir/glsl_types.cpp
+++ b/src/glsl/nir/glsl_types.cpp
@@ -124,6 +124,7 @@ glsl_type::glsl_type(const glsl_struct_field
*fields, unsigned num_fields,
   this->fields.structure[i].sample = fields[i].sample;
   this->fields.structure[i].matrix_layout = fields[i].matrix_layout;
   this->fields.structure[i].patch = fields[i].patch;
+  this->fields.structure[i].precision = fields[i].precision;
   this->fields.structure[i].image_read_only = fields[i].image_read_only;
   this->fields.structure[i].image_write_only = fields[i].image_write_only;
   this->fields.structure[i].image_coherent = fields[i].image_coherent;
diff --git a/src/glsl/nir/glsl_types.h b/src/glsl/nir/glsl_types.h
index d841a32..f3a0cf8 100644
--- a/src/glsl/nir/glsl_types.h
+++ b/src/glsl/nir/glsl_types.h
@@ -851,7 +851,7 @@ struct glsl_struct_field {

glsl_struct_field(const struct glsl_type *_type, const char *_name)
   : type(_type), name(_name), location(-1), interpolation(0), centroid(0),
-sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED), patch(0)
+sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED),
patch(0), precision(0)
{
   /* empty */
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-11 Thread Samuel Iglesias Gonsálvez 
Reviewed-by: Samuel Iglesias Gonsálvez 

On 06/11/15 13:03, Tapani Pälli wrote:
> From: Iago Toral Quiroga 
> 
> We will need this later on when we implement proper support for
> precision qualifiers in the drivers and also to do link time checks for
> uniforms as indicated by the spec.
> 
> This patch also adds compile-time checks for variables without precision
> information (currently, Mesa only checks that a default precision is set
> for floats in fragment shaders).
> 
> As indicated by Ian, the addition of the precision information to
> ir_variable has been done using a bitfield and pahole to identify an
> available hole so that memory requirements for ir_variable stay the
> same.
> 
> v2 (Ian):
>   - Avoid if-ladders by defining arrays of supported sampler names and
> indexing
> into them with type->sampler_array + 2 * type->sampler_shadow
>   - Make the code that selects the precision qualifier to use an utility
> function
>   - Fix a typo
> 
> v3 (Tapani):
>   - rebased
>   - squashed in "Precision qualifiers are not allowed on structs"
>   - fixed select_gles_precision for sampler arrays
>   - fixed precision_qualifier_allowed for arrays of structs
> 
> v4 (Tapani):
>   - add atomic_uint handling
>   - do not allow precision qualifier on images
>   (issues reported by Marta)
> 
> v5 (Tapani):
>   - support precision qualifier on image types
> ---
>  src/glsl/ast_to_hir.cpp | 296 
> 
>  src/glsl/ir.h   |  13 ++
>  src/glsl/nir/glsl_types.cpp |   4 +
>  src/glsl/nir/glsl_types.h   |  11 ++
>  4 files changed, 301 insertions(+), 23 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index b6d662b..1240615 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
>  * From this, we infer that GLSL 1.30 (and later) should allow precision
>  * qualifiers on sampler types just like float and integer types.
>  */
> -   return type->is_float()
> +   return (type->is_float()
> || type->is_integer()
> -   || type->is_record()
> -   || type->contains_opaque();
> +   || type->contains_opaque())
> +   && !type->without_array()->is_record();
>  }
>  
>  const glsl_type *
> @@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
> return type;
>  }
>  
> -const glsl_type *
> -ast_fully_specified_type::glsl_type(const char **name,
> -struct _mesa_glsl_parse_state *state) 
> const
> +/**
> + * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
> + *
> + * "The precision statement
> + *
> + *precision precision-qualifier type;
> + *
> + *  can be used to establish a default precision qualifier. The type field 
> can
> + *  be either int or float or any of the sampler types, (...) If type is 
> float,
> + *  the directive applies to non-precision-qualified floating point type
> + *  (scalar, vector, and matrix) declarations. If type is int, the directive
> + *  applies to all non-precision-qualified integer type (scalar, vector, 
> signed,
> + *  and unsigned) declarations."
> + *
> + * We use the symbol table to keep the values of the default precisions for
> + * each 'type' in each scope and we use the 'type' string from the precision
> + * statement as key in the symbol table. When we want to retrieve the default
> + * precision associated with a given glsl_type we need to know the type 
> string
> + * associated with it. This is what this function returns.
> + */
> +static const char *
> +get_type_name_for_precision_qualifier(const glsl_type *type)
>  {
> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
> -
> -   if (type == NULL)
> -  return NULL;
> +   switch (type->base_type) {
> +   case GLSL_TYPE_FLOAT:
> +  return "float";
> +   case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT:
> +  return "int";
> +   case GLSL_TYPE_ATOMIC_UINT:
> +  return "atomic_uint";
> +   case GLSL_TYPE_IMAGE:
> +   /* fallthrough */
> +   case GLSL_TYPE_SAMPLER: {
> +  const unsigned type_idx =
> + type->sampler_array + 2 * type->sampler_shadow;
> +  const unsigned offset = type->base_type == GLSL_TYPE_SAMPLER ? 0 : 4;
> +  assert(type_idx < 4);
> +  switch (type->sampler_type) {
> +  case GLSL_TYPE_FLOAT:
> + switch (type->sampler_dimensionality) {
> + case GLSL_SAMPLER_DIM_1D: {
> +assert(type->base_type == GLSL_TYPE_SAMPLER);
> +static const char *const names[4] = {
> +  "sampler1D", "sampler1DArray",
> +  "sampler1DShadow", "sampler1DArrayShadow"
> +};
> +return names[type_idx];
> + }
> + case GLSL_SAMPLER_DIM_2D: {
> +static const char *const names[8] = {
> +  "sampler2D", "sampler2DArray",
> +

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-10 Thread Tapani Pälli 



On 11/10/2015 12:26 PM, Iago Toral wrote:

On Fri, 2015-11-06 at 14:03 +0200, Tapani Pälli wrote:

From: Iago Toral Quiroga 

We will need this later on when we implement proper support for
precision qualifiers in the drivers and also to do link time checks for
uniforms as indicated by the spec.

This patch also adds compile-time checks for variables without precision
information (currently, Mesa only checks that a default precision is set
for floats in fragment shaders).

As indicated by Ian, the addition of the precision information to
ir_variable has been done using a bitfield and pahole to identify an
available hole so that memory requirements for ir_variable stay the
same.

v2 (Ian):
   - Avoid if-ladders by defining arrays of supported sampler names and
 indexing
 into them with type->sampler_array + 2 * type->sampler_shadow
   - Make the code that selects the precision qualifier to use an utility
 function
   - Fix a typo

v3 (Tapani):
   - rebased
   - squashed in "Precision qualifiers are not allowed on structs"
   - fixed select_gles_precision for sampler arrays
   - fixed precision_qualifier_allowed for arrays of structs

v4 (Tapani):
   - add atomic_uint handling
   - do not allow precision qualifier on images
   (issues reported by Marta)

v5 (Tapani):
   - support precision qualifier on image types
---
  src/glsl/ast_to_hir.cpp | 296 
  src/glsl/ir.h   |  13 ++
  src/glsl/nir/glsl_types.cpp |   4 +
  src/glsl/nir/glsl_types.h   |  11 ++
  4 files changed, 301 insertions(+), 23 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index b6d662b..1240615 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
  * From this, we infer that GLSL 1.30 (and later) should allow precision
  * qualifiers on sampler types just like float and integer types.
  */
-   return type->is_float()
+   return (type->is_float()
 || type->is_integer()
-   || type->is_record()
-   || type->contains_opaque();
+   || type->contains_opaque())
+   && !type->without_array()->is_record();
  }

  const glsl_type *
@@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
 return type;
  }

-const glsl_type *
-ast_fully_specified_type::glsl_type(const char **name,
-struct _mesa_glsl_parse_state *state) const
+/**
+ * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
+ *
+ * "The precision statement
+ *
+ *precision precision-qualifier type;
+ *
+ *  can be used to establish a default precision qualifier. The type field can
+ *  be either int or float or any of the sampler types, (...) If type is float,
+ *  the directive applies to non-precision-qualified floating point type
+ *  (scalar, vector, and matrix) declarations. If type is int, the directive
+ *  applies to all non-precision-qualified integer type (scalar, vector, 
signed,
+ *  and unsigned) declarations."
+ *
+ * We use the symbol table to keep the values of the default precisions for
+ * each 'type' in each scope and we use the 'type' string from the precision
+ * statement as key in the symbol table. When we want to retrieve the default
+ * precision associated with a given glsl_type we need to know the type string
+ * associated with it. This is what this function returns.
+ */
+static const char *
+get_type_name_for_precision_qualifier(const glsl_type *type)
  {
-   const struct glsl_type *type = this->specifier->glsl_type(name, state);
-
-   if (type == NULL)
-  return NULL;
+   switch (type->base_type) {
+   case GLSL_TYPE_FLOAT:
+  return "float";
+   case GLSL_TYPE_UINT:
+   case GLSL_TYPE_INT:
+  return "int";
+   case GLSL_TYPE_ATOMIC_UINT:
+  return "atomic_uint";
+   case GLSL_TYPE_IMAGE:
+   /* fallthrough */


I think this is not correct. As far as I understand the spec, we can set
a default precision for any of the image types:

image2D
image3D
imageCube
image2DArray
iimage2D
iimage3D
iimageCube
iimage2DArray
uimage2D
uimage3D
uimageCube
uimage2DArray

but here you are re-using the precisions from samplers, so if we do
this:

#precision lowp sampler2D;
#precision highp image2D;

the latter statement is ignored, and the former affects both samplers
and images, which is not what we want.


Note that we don't 'just fallthrough' here but we will return different 
values depending of type->base_type in the switch below. So for 
sampler2D, we will return 'sampler2D' and for image2D 'image2D' because 
of the offset set below:



Iago


+   case GLSL_TYPE_SAMPLER: {
+  const unsigned type_idx =
+ type->sampler_array + 2 * type->sampler_shadow;
+  const unsigned offset = type->base_type == GLSL_TYPE_SAMPLER ? 0 : 4;
+  assert(type_idx < 4);
+  switch (type->sampler_type) {
+  case GLSL_TYPE_FLOAT:
+

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-10 Thread Iago Toral 
On Fri, 2015-11-06 at 14:03 +0200, Tapani Pälli wrote:
> From: Iago Toral Quiroga 
> 
> We will need this later on when we implement proper support for
> precision qualifiers in the drivers and also to do link time checks for
> uniforms as indicated by the spec.
> 
> This patch also adds compile-time checks for variables without precision
> information (currently, Mesa only checks that a default precision is set
> for floats in fragment shaders).
> 
> As indicated by Ian, the addition of the precision information to
> ir_variable has been done using a bitfield and pahole to identify an
> available hole so that memory requirements for ir_variable stay the
> same.
> 
> v2 (Ian):
>   - Avoid if-ladders by defining arrays of supported sampler names and
> indexing
> into them with type->sampler_array + 2 * type->sampler_shadow
>   - Make the code that selects the precision qualifier to use an utility
> function
>   - Fix a typo
> 
> v3 (Tapani):
>   - rebased
>   - squashed in "Precision qualifiers are not allowed on structs"
>   - fixed select_gles_precision for sampler arrays
>   - fixed precision_qualifier_allowed for arrays of structs
> 
> v4 (Tapani):
>   - add atomic_uint handling
>   - do not allow precision qualifier on images
>   (issues reported by Marta)
> 
> v5 (Tapani):
>   - support precision qualifier on image types
> ---
>  src/glsl/ast_to_hir.cpp | 296 
> 
>  src/glsl/ir.h   |  13 ++
>  src/glsl/nir/glsl_types.cpp |   4 +
>  src/glsl/nir/glsl_types.h   |  11 ++
>  4 files changed, 301 insertions(+), 23 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index b6d662b..1240615 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
>  * From this, we infer that GLSL 1.30 (and later) should allow precision
>  * qualifiers on sampler types just like float and integer types.
>  */
> -   return type->is_float()
> +   return (type->is_float()
> || type->is_integer()
> -   || type->is_record()
> -   || type->contains_opaque();
> +   || type->contains_opaque())
> +   && !type->without_array()->is_record();
>  }
>  
>  const glsl_type *
> @@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
> return type;
>  }
>  
> -const glsl_type *
> -ast_fully_specified_type::glsl_type(const char **name,
> -struct _mesa_glsl_parse_state *state) 
> const
> +/**
> + * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
> + *
> + * "The precision statement
> + *
> + *precision precision-qualifier type;
> + *
> + *  can be used to establish a default precision qualifier. The type field 
> can
> + *  be either int or float or any of the sampler types, (...) If type is 
> float,
> + *  the directive applies to non-precision-qualified floating point type
> + *  (scalar, vector, and matrix) declarations. If type is int, the directive
> + *  applies to all non-precision-qualified integer type (scalar, vector, 
> signed,
> + *  and unsigned) declarations."
> + *
> + * We use the symbol table to keep the values of the default precisions for
> + * each 'type' in each scope and we use the 'type' string from the precision
> + * statement as key in the symbol table. When we want to retrieve the default
> + * precision associated with a given glsl_type we need to know the type 
> string
> + * associated with it. This is what this function returns.
> + */
> +static const char *
> +get_type_name_for_precision_qualifier(const glsl_type *type)
>  {
> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
> -
> -   if (type == NULL)
> -  return NULL;
> +   switch (type->base_type) {
> +   case GLSL_TYPE_FLOAT:
> +  return "float";
> +   case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT:
> +  return "int";
> +   case GLSL_TYPE_ATOMIC_UINT:
> +  return "atomic_uint";
> +   case GLSL_TYPE_IMAGE:
> +   /* fallthrough */

I think this is not correct. As far as I understand the spec, we can set
a default precision for any of the image types:

image2D
image3D
imageCube
image2DArray
iimage2D
iimage3D
iimageCube
iimage2DArray
uimage2D
uimage3D
uimageCube
uimage2DArray

but here you are re-using the precisions from samplers, so if we do
this:

#precision lowp sampler2D;
#precision highp image2D;

the latter statement is ignored, and the former affects both samplers
and images, which is not what we want.

Iago

> +   case GLSL_TYPE_SAMPLER: {
> +  const unsigned type_idx =
> + type->sampler_array + 2 * type->sampler_shadow;
> +  const unsigned offset = type->base_type == GLSL_TYPE_SAMPLER ? 0 : 4;
> +  assert(type_idx < 4);
> +  switch (type->sampler_type) {
> +  case GLSL_TYPE_FLOAT:
> + switch (type->sampler_dimensionality) {
> + case

Re: [Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-10 Thread Iago Toral 
On Tue, 2015-11-10 at 12:41 +0200, Tapani Pälli wrote:
> 
> On 11/10/2015 12:26 PM, Iago Toral wrote:
> > On Fri, 2015-11-06 at 14:03 +0200, Tapani Pälli wrote:
> >> From: Iago Toral Quiroga 
> >>
> >> We will need this later on when we implement proper support for
> >> precision qualifiers in the drivers and also to do link time checks for
> >> uniforms as indicated by the spec.
> >>
> >> This patch also adds compile-time checks for variables without precision
> >> information (currently, Mesa only checks that a default precision is set
> >> for floats in fragment shaders).
> >>
> >> As indicated by Ian, the addition of the precision information to
> >> ir_variable has been done using a bitfield and pahole to identify an
> >> available hole so that memory requirements for ir_variable stay the
> >> same.
> >>
> >> v2 (Ian):
> >>- Avoid if-ladders by defining arrays of supported sampler names and
> >>  indexing
> >>  into them with type->sampler_array + 2 * type->sampler_shadow
> >>- Make the code that selects the precision qualifier to use an utility
> >>  function
> >>- Fix a typo
> >>
> >> v3 (Tapani):
> >>- rebased
> >>- squashed in "Precision qualifiers are not allowed on structs"
> >>- fixed select_gles_precision for sampler arrays
> >>- fixed precision_qualifier_allowed for arrays of structs
> >>
> >> v4 (Tapani):
> >>- add atomic_uint handling
> >>- do not allow precision qualifier on images
> >>(issues reported by Marta)
> >>
> >> v5 (Tapani):
> >>- support precision qualifier on image types
> >> ---
> >>   src/glsl/ast_to_hir.cpp | 296 
> >> 
> >>   src/glsl/ir.h   |  13 ++
> >>   src/glsl/nir/glsl_types.cpp |   4 +
> >>   src/glsl/nir/glsl_types.h   |  11 ++
> >>   4 files changed, 301 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> >> index b6d662b..1240615 100644
> >> --- a/src/glsl/ast_to_hir.cpp
> >> +++ b/src/glsl/ast_to_hir.cpp
> >> @@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
> >>   * From this, we infer that GLSL 1.30 (and later) should allow 
> >> precision
> >>   * qualifiers on sampler types just like float and integer types.
> >>   */
> >> -   return type->is_float()
> >> +   return (type->is_float()
> >>  || type->is_integer()
> >> -   || type->is_record()
> >> -   || type->contains_opaque();
> >> +   || type->contains_opaque())
> >> +   && !type->without_array()->is_record();
> >>   }
> >>
> >>   const glsl_type *
> >> @@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
> >>  return type;
> >>   }
> >>
> >> -const glsl_type *
> >> -ast_fully_specified_type::glsl_type(const char **name,
> >> -struct _mesa_glsl_parse_state *state) 
> >> const
> >> +/**
> >> + * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
> >> + *
> >> + * "The precision statement
> >> + *
> >> + *precision precision-qualifier type;
> >> + *
> >> + *  can be used to establish a default precision qualifier. The type 
> >> field can
> >> + *  be either int or float or any of the sampler types, (...) If type is 
> >> float,
> >> + *  the directive applies to non-precision-qualified floating point type
> >> + *  (scalar, vector, and matrix) declarations. If type is int, the 
> >> directive
> >> + *  applies to all non-precision-qualified integer type (scalar, vector, 
> >> signed,
> >> + *  and unsigned) declarations."
> >> + *
> >> + * We use the symbol table to keep the values of the default precisions 
> >> for
> >> + * each 'type' in each scope and we use the 'type' string from the 
> >> precision
> >> + * statement as key in the symbol table. When we want to retrieve the 
> >> default
> >> + * precision associated with a given glsl_type we need to know the type 
> >> string
> >> + * associated with it. This is what this function returns.
> >> + */
> >> +static const char *
> >> +get_type_name_for_precision_qualifier(const glsl_type *type)
> >>   {
> >> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
> >> -
> >> -   if (type == NULL)
> >> -  return NULL;
> >> +   switch (type->base_type) {
> >> +   case GLSL_TYPE_FLOAT:
> >> +  return "float";
> >> +   case GLSL_TYPE_UINT:
> >> +   case GLSL_TYPE_INT:
> >> +  return "int";
> >> +   case GLSL_TYPE_ATOMIC_UINT:
> >> +  return "atomic_uint";
> >> +   case GLSL_TYPE_IMAGE:
> >> +   /* fallthrough */
> >
> > I think this is not correct. As far as I understand the spec, we can set
> > a default precision for any of the image types:
> >
> > image2D
> > image3D
> > imageCube
> > image2DArray
> > iimage2D
> > iimage3D
> > iimageCube
> > iimage2DArray
> > uimage2D
> > uimage3D
> > uimageCube
> > uimage2DArray
> >
> > but here you are re-using the precisions from samplers, so if we do
> > this:
>

[Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

2015-11-06 Thread Tapani Pälli 
From: Iago Toral Quiroga 

We will need this later on when we implement proper support for
precision qualifiers in the drivers and also to do link time checks for
uniforms as indicated by the spec.

This patch also adds compile-time checks for variables without precision
information (currently, Mesa only checks that a default precision is set
for floats in fragment shaders).

As indicated by Ian, the addition of the precision information to
ir_variable has been done using a bitfield and pahole to identify an
available hole so that memory requirements for ir_variable stay the
same.

v2 (Ian):
  - Avoid if-ladders by defining arrays of supported sampler names and
indexing
into them with type->sampler_array + 2 * type->sampler_shadow
  - Make the code that selects the precision qualifier to use an utility
function
  - Fix a typo

v3 (Tapani):
  - rebased
  - squashed in "Precision qualifiers are not allowed on structs"
  - fixed select_gles_precision for sampler arrays
  - fixed precision_qualifier_allowed for arrays of structs

v4 (Tapani):
  - add atomic_uint handling
  - do not allow precision qualifier on images
  (issues reported by Marta)

v5 (Tapani):
  - support precision qualifier on image types
---
 src/glsl/ast_to_hir.cpp | 296 
 src/glsl/ir.h   |  13 ++
 src/glsl/nir/glsl_types.cpp |   4 +
 src/glsl/nir/glsl_types.h   |  11 ++
 4 files changed, 301 insertions(+), 23 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index b6d662b..1240615 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
 * From this, we infer that GLSL 1.30 (and later) should allow precision
 * qualifiers on sampler types just like float and integer types.
 */
-   return type->is_float()
+   return (type->is_float()
|| type->is_integer()
-   || type->is_record()
-   || type->contains_opaque();
+   || type->contains_opaque())
+   && !type->without_array()->is_record();
 }
 
 const glsl_type *
@@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
return type;
 }
 
-const glsl_type *
-ast_fully_specified_type::glsl_type(const char **name,
-struct _mesa_glsl_parse_state *state) const
+/**
+ * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
+ *
+ * "The precision statement
+ *
+ *precision precision-qualifier type;
+ *
+ *  can be used to establish a default precision qualifier. The type field can
+ *  be either int or float or any of the sampler types, (...) If type is float,
+ *  the directive applies to non-precision-qualified floating point type
+ *  (scalar, vector, and matrix) declarations. If type is int, the directive
+ *  applies to all non-precision-qualified integer type (scalar, vector, 
signed,
+ *  and unsigned) declarations."
+ *
+ * We use the symbol table to keep the values of the default precisions for
+ * each 'type' in each scope and we use the 'type' string from the precision
+ * statement as key in the symbol table. When we want to retrieve the default
+ * precision associated with a given glsl_type we need to know the type string
+ * associated with it. This is what this function returns.
+ */
+static const char *
+get_type_name_for_precision_qualifier(const glsl_type *type)
 {
-   const struct glsl_type *type = this->specifier->glsl_type(name, state);
-
-   if (type == NULL)
-  return NULL;
+   switch (type->base_type) {
+   case GLSL_TYPE_FLOAT:
+  return "float";
+   case GLSL_TYPE_UINT:
+   case GLSL_TYPE_INT:
+  return "int";
+   case GLSL_TYPE_ATOMIC_UINT:
+  return "atomic_uint";
+   case GLSL_TYPE_IMAGE:
+   /* fallthrough */
+   case GLSL_TYPE_SAMPLER: {
+  const unsigned type_idx =
+ type->sampler_array + 2 * type->sampler_shadow;
+  const unsigned offset = type->base_type == GLSL_TYPE_SAMPLER ? 0 : 4;
+  assert(type_idx < 4);
+  switch (type->sampler_type) {
+  case GLSL_TYPE_FLOAT:
+ switch (type->sampler_dimensionality) {
+ case GLSL_SAMPLER_DIM_1D: {
+assert(type->base_type == GLSL_TYPE_SAMPLER);
+static const char *const names[4] = {
+  "sampler1D", "sampler1DArray",
+  "sampler1DShadow", "sampler1DArrayShadow"
+};
+return names[type_idx];
+ }
+ case GLSL_SAMPLER_DIM_2D: {
+static const char *const names[8] = {
+  "sampler2D", "sampler2DArray",
+  "sampler2DShadow", "sampler2DArrayShadow",
+  "image2D", "image2DArray", NULL, NULL
+};
+return names[offset + type_idx];
+ }
+ case GLSL_SAMPLER_DIM_3D: {
+static const char *const names[8] = {
+  "sampler3D", NULL, NULL, NULL,
+  "image3D", NULL, NULL, NULL
+