[Mesa-dev] [PATCH 11/21] glsl: Store ir_variable::ir_type in 8 bits instead of 32

[Ian Romanick]

On 05/27/2014 08:28 PM, Matt Turner wrote:
> On Tue, May 27, 2014 at 7:49 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> No change in the peak ir_variable memory usage in a trimmed apitrace of
>> dota2 on 64-bit.
>>
>> No change in the peak ir_variable memory usage in a trimmed apitrace of
>> dota2 on 32-bit.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/glsl/ir.h | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
>> index 7faee74..bc02f6e 100644
>> --- a/src/glsl/ir.h
>> +++ b/src/glsl/ir.h
>> @@ -92,12 +92,13 @@ enum ir_node_type {
>>   */
>>  class ir_instruction : public exec_node {
>>  private:
>> -   enum ir_node_type ir_type;
>> +   uint8_t ir_type;
>>
>>  public:
>>     inline enum ir_node_type get_ir_type() const
>>     {
>> -      return this->ir_type;
>> +      STATIC_ASSERT(ir_type_max < 256);
>> +      return (enum ir_node_type) this->ir_type;
>>     }
>>
>>     /**
>> --
>> 1.8.1.4
> 
> Instead of doing this, you can mark the enum type with the PACKED
> attribute. I did this in a similar change in i965 already. See
> http://lists.freedesktop.org/archives/mesa-dev/2014-February/054643.html
> 
> This way we still get enum type checking and warnings out of switch
> statements and such.

Hmm... that would mean that patch 10 wouldn't strictly be necessary.
The disadvantage is that the next patch would need (right?) some changes
for MSVC, especially on 32-bit.  I think it would need to be

#if sizeof(ir_node_type) < sizeof(void *)
# define PADDING_BYTES (sizeof(void *) - sizeof(ir_node_type))
#else
# define PADDING_BYTES sizeof(void *)
#  if (__GNUC__ >= 3)
#    error "GCC did us wrong."
#  endif
#endif

  uint8_t padding[PADDING_BYTES];

Seems a little sketchy, but might still be better... hmm...