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

[Ian Romanick]

On 05/28/2014 12:20 PM, Patrick Baggett wrote:
> On Wed, May 28, 2014 at 2:17 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     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
>     <mailto:idr at freedesktop.org>> wrote:
>     >> From: Ian Romanick <ian.d.romanick at intel.com
>     <mailto: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
>     <mailto: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 *)
> 
> 
> I don't think the preprocessor can evaluate sizeof().

Yes, of course.  Ugh. :(

>     # 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...
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/mesa-dev