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,
[...]
};


-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,




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

Reply via email to