[Mesa-dev] [PATCH] glsl: s/unsigned/glsl_base_type/ in glsl type code

[Brian Paul]

On 11/06/2017 02:27 PM, Ian Romanick wrote:
> On 11/06/2017 01:00 PM, Brian Paul wrote:
>> Declare glsl_type::sampled_type as glsl_base_type as we do for the
>> base_type field.  And make base_type a bitfield to save a few bytes.
>
> Hmm... I have mixed feelings about this.  I made a conscious decision to
> have base_type be "full size" because it's used a lot.  I suspect there
> will be some increase in code size across this change.  There's probably
> also some performance difference, but it may not be enough to be
> measurable.  I do like actually using type names. :)
>
> As new base types were added, sampled_type remained 2 bits because GLSL
> only allows float, int and uint.  This is the reason GLSL_TYPE_UINT64
> and GLSL_TYPE_INT64 are not grouped with GLSL_TYPE_UINT and GLSL_TYPE_INT.
>
> I wonder if it might be more compact (in terms of generated code) to
> make both fields 8 bits and group them together.

Probably.  I can do that in a v2.  Otherwise, I'm fine with leaving 
base_type unchanged.  Your call.

-Brian


>
>> Update glsl_type constructor to take glsl_base_type intead of unsigned
>                                                        ^^^^^^ instead
>
>> and pass GLSL_TYPE_VOID instead of zero.
>>
>> No Piglit regressions with llvmpipe.
>> ---
>>   src/compiler/glsl_types.cpp | 14 +++++++-------
>>   src/compiler/glsl_types.h   | 14 +++++++-------
>>   2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
>> index 704b63c..1d20b02 100644
>> --- a/src/compiler/glsl_types.cpp
>> +++ b/src/compiler/glsl_types.cpp
>> @@ -52,7 +52,7 @@ glsl_type::glsl_type(GLenum gl_type,
>>      gl_type(gl_type),
>>      base_type(base_type),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing(0), interface_row_major(0),
>>      vector_elements(vector_elements), matrix_columns(matrix_columns),
>>      length(0)
>>   {
>> @@ -79,7 +79,7 @@ glsl_type::glsl_type(GLenum gl_type,
>>
>>   glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type,
>>                        enum glsl_sampler_dim dim, bool shadow, bool array,
>> -                     unsigned type, const char *name) :
>> +                     glsl_base_type type, const char *name) :
>>      gl_type(gl_type),
>>      base_type(base_type),
>>      sampler_dimensionality(dim), sampler_shadow(shadow),
>> @@ -104,7 +104,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>      gl_type(0),
>>      base_type(GLSL_TYPE_STRUCT),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing(0), interface_row_major(0),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_fields)
>>   {
>> @@ -133,7 +133,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>      gl_type(0),
>>      base_type(GLSL_TYPE_INTERFACE),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing((unsigned) packing),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing((unsigned) packing),
>>      interface_row_major((unsigned) row_major),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_fields)
>> @@ -161,7 +161,7 @@ glsl_type::glsl_type(const glsl_type *return_type,
>>      gl_type(0),
>>      base_type(GLSL_TYPE_FUNCTION),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing(0), interface_row_major(0),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_params)
>>   {
>> @@ -193,7 +193,7 @@ glsl_type::glsl_type(const char *subroutine_name) :
>>      gl_type(0),
>>      base_type(GLSL_TYPE_SUBROUTINE),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing(0), interface_row_major(0),
>>      vector_elements(1), matrix_columns(1),
>>      length(0)
>>   {
>> @@ -444,7 +444,7 @@ _mesa_glsl_release_types(void)
>>   glsl_type::glsl_type(const glsl_type *array, unsigned length) :
>>      base_type(GLSL_TYPE_ARRAY),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   sampled_type(GLSL_TYPE_VOID), interface_packing(0), interface_row_major(0),
>>      vector_elements(0), matrix_columns(0),
>>      length(length), name(NULL)
>>   {
>> diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
>> index 0b4a66c..6e2d6cc 100644
>> --- a/src/compiler/glsl_types.h
>> +++ b/src/compiler/glsl_types.h
>> @@ -145,16 +145,16 @@ enum {
>>
>>   struct glsl_type {
>>      GLenum gl_type;
>> -   glsl_base_type base_type;
>> +   glsl_base_type base_type:6;
>>
>>      unsigned sampler_dimensionality:4; /**< \see glsl_sampler_dim */
>>      unsigned sampler_shadow:1;
>>      unsigned sampler_array:1;
>> -   unsigned sampled_type:2;    /**< Type of data returned using this
>> -				* sampler or image.  Only \c
>> -				* GLSL_TYPE_FLOAT, \c GLSL_TYPE_INT,
>> -				* and \c GLSL_TYPE_UINT are valid.
>> -				*/
>> +   glsl_base_type sampled_type:6; /**< Type of data returned using this
>> +                                   * sampler or image.  Only \c
>> +                                   * GLSL_TYPE_FLOAT, \c GLSL_TYPE_INT,
>> +                                   * and \c GLSL_TYPE_UINT are valid.
>> +                                   */
>>      unsigned interface_packing:2;
>>      unsigned interface_row_major:1;
>>
>> @@ -874,7 +874,7 @@ private:
>>      /** Constructor for sampler or image types */
>>      glsl_type(GLenum gl_type, glsl_base_type base_type,
>>   	     enum glsl_sampler_dim dim, bool shadow, bool array,
>> -	     unsigned type, const char *name);
>> +	     glsl_base_type type, const char *name);
>>
>>      /** Constructor for record types */
>>      glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>
>