Jose, Brian,

> Marc,
> 
> Why is this necessary? It has been working fine so far. Which gcc version
>  are you using? What commas are you referring to?

the PIPE_ALIGN_TYPE macro is so far only used in the cell driver in 
src/gallium/drivers/cell/spu/spu_main.c  (this is probably why no one noticed 
it).

The marco takes a type, a stuct in this case, which can include commas:

PIPE_ALIGN_TYPE(16,
struct spu_framebuffer
{
   void *color_start;              /**< addr of color surface in main memory */
   void *depth_start;              /**< addr of depth surface in main memory */
   enum pipe_format color_format;
   enum pipe_format depth_format;
   uint width, height;             /**< size in pixels */
            ^^^

   uint width_tiles, height_tiles; /**< width and height in tiles */
                  ^^^

   uint color_clear_value;
   uint depth_clear_value;

   uint zsize;                     /**< 0, 2 or 4 bytes per Z */
   float zscale;                   /**< 65535.0, 2^24-1 or 2^32-1 */
});

This will cause a problem, as the macro will thread each comma as an argument 
seperator and thus the number of arguments is larger than 2.

> Variadic macros aren't portable. This change would break MSVC build.

As it is only used by cell for now, the msvc macro does not need to be changed. 
But 
there should be some portable solution.

> It seems that there is some code that's trying to pass more than 2 args to
>  PIPE_ALIGN_TYPE. That sounds wrong. And making PIPE_ALIGN_TYPE take a
>  variable number of args just a bandaid for a deeper problem.

I was looking for the least invasive solution and found this (I'm not a C 
coder).
The other method would be to concatenate the "struct" first by a second macro 
to a 
single argument, but I don't know how to do this.

I read that variadic macros are part of ISO C99, but the form I used is 
probably a 
gcc extension. Maybe

#define PIPE_ALIGN_TYPE(_alignment,
...) __VA_ARGS__ __attribute__((aligned(_alignment)))

is more portable.

Marc

> Jose
> 
> ________________________________________
> From: Brian Paul [bri...@vmware.com]
> Sent: Tuesday, January 26, 2010 0:05
> To: Marc Dietrich
> Cc: mesa3d-dev@lists.sourceforge.net
> Subject: Re: [Mesa3d-dev] [PATCH] hack around commas in macro argument
> 
> Marc Dietrich wrote:
> > this is needed at least for gcc. dunno about the other compilers.
> > ---
> >  src/gallium/include/pipe/p_compiler.h |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/include/pipe/p_compiler.h
> > b/src/gallium/include/pipe/p_compiler.h index 272d030..cdda20c 100644
> > --- a/src/gallium/include/pipe/p_compiler.h
> > +++ b/src/gallium/include/pipe/p_compiler.h
> > @@ -144,7 +144,7 @@ typedef unsigned char boolean;
> >  #if defined(__GNUC__) || (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))
> >
> >  /* See http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Type-Attributes.html
> > */ -#define PIPE_ALIGN_TYPE(_alignment, _type) _type
> > __attribute__((aligned(_alignment))) +#define PIPE_ALIGN_TYPE(_alignment,
> > _type...) _type __attribute__((aligned(_alignment)))
> >
> >  /* See
> > http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Variable-Attributes.html */
> > #define PIPE_ALIGN_VAR(_alignment) __attribute__((aligned(_alignment)))
> > @@ -158,14 +158,14 @@ typedef unsigned char boolean;
> >  #elif defined(_MSC_VER)
> >
> >  /* See http://msdn.microsoft.com/en-us/library/83ythb65.aspx */
> > -#define PIPE_ALIGN_TYPE(_alignment, _type) __declspec(align(_alignment))
> > _type +#define PIPE_ALIGN_TYPE(_alignment, _type...)
> > __declspec(align(_alignment)) _type #define PIPE_ALIGN_VAR(_alignment)
> > __declspec(align(_alignment))
> >
> >  #define PIPE_ALIGN_STACK
> >
> >  #elif defined(SWIG)
> >
> > -#define PIPE_ALIGN_TYPE(_alignment, _type) _type
> > +#define PIPE_ALIGN_TYPE(_alignment, _type...) _type
> >  #define PIPE_ALIGN_VAR(_alignment)
> >
> >  #define PIPE_ALIGN_STACK
> 
> With your patch I'm seeing new warnings that I didn't get before:
> 
> ../../src/gallium/include/pipe/p_compiler.h:147:42: warning: ISO C
> does not permit named variadic macros
> 
> I'm using gcc 4.3.2
> 
> Also, if you didn't test other compilers, the patch probably should
> not change PIPE_ALIGN_TYPE for MSVC or SWIG, right?
> 
> -Brian
> 
> ---------------------------------------------------------------------------
> --- The Planet: dedicated and managed hosting, cloud storage, colocation
>  Stay online with enterprise data centers and the best network in the
>  business Choose flexible plans and management services without long-term
>  contracts Personal 24x7 support from experience hosting pros just a phone
>  call away. http://p.sf.net/sfu/theplanet-com
> _______________________________________________
> Mesa3d-dev mailing list
> Mesa3d-dev@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
> 

------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to