[Mesa-dev] [PATCH 3/4] glsl: s/unsigned/glsl_base_type/ in glsl type code (v2)
Brian Paul
brianp at vmware.com
Fri Nov 10 17:28:05 UTC 2017
On 11/08/2017 08:27 PM, Brian Paul wrote:
> On 11/08/2017 08:12 PM, Brian Paul wrote:
>> 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?
>
>
> FWIW: here's what it would basically look like:
>
> #ifndef __cplusplus
> #error This only works with C++
> #endif
>
> #if defined(_MSC_VER)
> #define ENUM_8BIT(NAME) enum NAME : unsigned char
> #elif defined(__GNUC__)
> #define ENUM_8BIT(NAME) enum __attribute__((__packed__)) NAME
> #else
> #define ENUM_8BIT(NAME) enum NAME
> #endif
>
> ENUM_8BIT(glsl_base_type) {
> GLSL_TYPE_UINT = 0,
> GLSL_TYPE_INT,
> [...]
> };
Ian, any more comments on this? Does the 4-patch series look OK as-is?
I can implement the ENUM_8BIT() thing in a follow-on.
-Brian
>
> -Brian
>
>
>>
>> -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