[Mesa-dev] [PATCH 3/4] glsl: s/unsigned/glsl_base_type/ in glsl type code (v2)

Brian Paul brianp at vmware.com
Thu Nov 9 03:12:00 UTC 2017


On 11/08/2017 06:28 PM, Ian Romanick wrote:
> Any thoughts about my data using __attribute__((__packed__))?

Sorry, I didn't have time to dig into it.  I took a look this evening.

I think the ENUM_8BIT idea will work for GCC and MSVC but only for C++ 
sources.  MSVC doesn't like the sized enum syntax in C compilation units 
(unless there's some compiler flag I haven't found yet).  So, we could 
use it in the GLSL compiler, but not over in the gallium headers.

Does that matter to you?

Could I address this issue in a follow-on after the current series?

-Brian

>
> On 11/07/2017 04:07 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.
>>
>> Update glsl_type constructor to take glsl_base_type instead of unsigned
>> and pass GLSL_TYPE_VOID instead of zero.
>>
>> No Piglit regressions with llvmpipe.
>>
>> v2:
>> - Declare both base_type and sampled_type as 8-bit fields
>> - Use the new ASSERT_BITFIELD_SIZE() macro.
>> ---
>>   src/compiler/glsl_types.cpp | 30 +++++++++++++++---------------
>>   src/compiler/glsl_types.h   | 28 +++++++++++++++++++++-------
>>   2 files changed, 36 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
>> index 704b63c..107a81f 100644
>> --- a/src/compiler/glsl_types.cpp
>> +++ b/src/compiler/glsl_types.cpp
>> @@ -50,9 +50,9 @@ glsl_type::glsl_type(GLenum gl_type,
>>                        glsl_base_type base_type, unsigned vector_elements,
>>                        unsigned matrix_columns, const char *name) :
>>      gl_type(gl_type),
>> -   base_type(base_type),
>> +   base_type(base_type), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   interface_packing(0), interface_row_major(0),
>>      vector_elements(vector_elements), matrix_columns(matrix_columns),
>>      length(0)
>>   {
>> @@ -79,11 +79,11 @@ 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),
>> +   base_type(base_type), sampled_type(type),
>>      sampler_dimensionality(dim), sampler_shadow(shadow),
>> -   sampler_array(array), sampled_type(type), interface_packing(0),
>> +   sampler_array(array), interface_packing(0),
>>      interface_row_major(0), length(0)
>>   {
>>      mtx_lock(&glsl_type::mem_mutex);
>> @@ -102,9 +102,9 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type,
>>   glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>                        const char *name) :
>>      gl_type(0),
>> -   base_type(GLSL_TYPE_STRUCT),
>> +   base_type(GLSL_TYPE_STRUCT), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   interface_packing(0), interface_row_major(0),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_fields)
>>   {
>> @@ -131,9 +131,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>                        enum glsl_interface_packing packing,
>>                        bool row_major, const char *name) :
>>      gl_type(0),
>> -   base_type(GLSL_TYPE_INTERFACE),
>> +   base_type(GLSL_TYPE_INTERFACE), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing((unsigned) packing),
>> +   interface_packing((unsigned) packing),
>>      interface_row_major((unsigned) row_major),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_fields)
>> @@ -159,9 +159,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>>   glsl_type::glsl_type(const glsl_type *return_type,
>>                        const glsl_function_param *params, unsigned num_params) :
>>      gl_type(0),
>> -   base_type(GLSL_TYPE_FUNCTION),
>> +   base_type(GLSL_TYPE_FUNCTION), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   interface_packing(0), interface_row_major(0),
>>      vector_elements(0), matrix_columns(0),
>>      length(num_params)
>>   {
>> @@ -191,9 +191,9 @@ glsl_type::glsl_type(const glsl_type *return_type,
>>
>>   glsl_type::glsl_type(const char *subroutine_name) :
>>      gl_type(0),
>> -   base_type(GLSL_TYPE_SUBROUTINE),
>> +   base_type(GLSL_TYPE_SUBROUTINE), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   interface_packing(0), interface_row_major(0),
>>      vector_elements(1), matrix_columns(1),
>>      length(0)
>>   {
>> @@ -442,9 +442,9 @@ _mesa_glsl_release_types(void)
>>
>>
>>   glsl_type::glsl_type(const glsl_type *array, unsigned length) :
>> -   base_type(GLSL_TYPE_ARRAY),
>> +   base_type(GLSL_TYPE_ARRAY), sampled_type(GLSL_TYPE_VOID),
>>      sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
>> -   sampled_type(0), interface_packing(0), interface_row_major(0),
>> +   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..3a3d3d8 100644
>> --- a/src/compiler/glsl_types.h
>> +++ b/src/compiler/glsl_types.h
>> @@ -28,6 +28,7 @@
>>   #include <string.h>
>>   #include <assert.h>
>>
>> +#include "util/bitfield_assert.h"
>>   #include "shader_enums.h"
>>   #include "blob.h"
>>
>> @@ -145,23 +146,36 @@ enum {
>>
>>   struct glsl_type {
>>      GLenum gl_type;
>> -   glsl_base_type base_type;
>> +   glsl_base_type base_type:8;
>> +
>> +   glsl_base_type sampled_type:8; /**< 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 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.
>> -				*/
>>      unsigned interface_packing:2;
>>      unsigned interface_row_major:1;
>>
>> +private:
>> +   glsl_type()
>> +   {
>> +      // Dummy constructor, just for the sake of ASSERT_BITFIELD_SIZE.
>> +   }
>> +
>> +public:
>>      /* Callers of this ralloc-based new need not call delete. It's
>>       * easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */
>>      static void* operator new(size_t size)
>>      {
>> +      ASSERT_BITFIELD_SIZE(glsl_type, base_type, GLSL_TYPE_ERROR);
>> +      ASSERT_BITFIELD_SIZE(glsl_type, sampled_type, GLSL_TYPE_ERROR);
>> +      ASSERT_BITFIELD_SIZE(glsl_type, sampler_dimensionality,
>> +                           GLSL_SAMPLER_DIM_SUBPASS_MS);
>> +
>>         mtx_lock(&glsl_type::mem_mutex);
>>
>>         /* mem_ctx should have been created by the static members */
>> @@ -874,7 +888,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,
>>
>



More information about the mesa-dev mailing list