Re: __attribute__ for non-gcc compilers

On 2015-01-14 15:48:47 -0500, Robert Haas wrote:
> On Tue, Jan 13, 2015 at 4:18 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
> > Commit db4ec2ffce35 added alignment attributes for 64-bit atomic
> > variables as required on 32-bit platforms using
> > __attribute__((aligned(8)). That works fine with GCC, and would work
> > with Solaris Studio Compiler [1] and IBM XL C [2], but src/include/c.h
> > defines __attribute__ as an empty macro when not using GCC.
> > Unfortunately we can't just disable that #define and enable all
> > __attributes__ for Solaris CC and XLC as we use a bunch of attributes
> > that are not supported by those compilers and using them unconditionally
> > would generate a lot of warnings.
> >
> > Attached a patch that defines custom macros for each attribute and
> > enables them individually for compilers that support them and never
> > defines __attribute__.
> >
> > I have tested this with GCC 4.9.2 on Linux x86-64 and Solaris CC 5.12 on
> > Sparc (32-bit build); I don't have access to an IBM box with XLC.
>
> I guess my first question is whether we want to be relying on
> __attribute__((aligned)) in the first place.

I think it's better than the alternatives:

a) Don't support 64bit atomics on any 32bit platform. I think that'd be
sad because there's some places that could greatly benefit from being
able to read/store/manipulate e.g. LSNs atomically.
b) Double the size of 64bit atomics on 32bit platforms, and add
TYPEALIGN() to every access inside the atomics implementation.
c) Require 64 atomics to be aligned appropriately manually in every
place they're embedded. I think that's completely impractical.

The only viable one imo is a)

Since the atomics code is compiler dependant anyway, I don't much of a
problem with relying on compiler specific alignment
instructions. Really, the only problem right now is that __attribute__
is defined to be empty on compilers where it has meaning.

> If we do, then this seems like a pretty sensible and necessary change.

I think it's also an improvment independent from the alignment
code. Having a more widely portable __attribute__((noreturn)),
__attribute__((unused)), __attribute__((format(..))) seems like a good
general improvement. And all of these are supported in other compilers
than gcc.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services