On 2013-10-07 13:25:17 -0400, Robert Haas wrote:
> On Fri, Oct 4, 2013 at 8:19 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> > Could it be that MAXALIGN/TYPEALIGN doesn't really work for values
> > bigger than 32bit?
> >
> > #define MAXALIGN(LEN) TYPEALIGN(MAXIMUM_ALIGNOF, (LEN))
> > #define TYPEALIGN(ALIGNVAL,LEN) \
> > (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL) - 1)))
>
> Isn't the problem, more specifically, that it doesn't work for values
> larger than an intptr_t?
Well, yes. And intptr_t is 32bit wide on a 32bit platform.
> And does that indicate that intptr_t is the wrong type to be using here?
No, I don't think so. intptr_t is defined to be a integer type to which
you can cast a pointer, cast it back and still get the old value. On
32bit platforms it usually will be 32bit wide.
All that's fine for the classic usages of TYPEALIGN where it's used on
pointers or lengths of stuff stored in memory. Those will always fit in
32bit on a 32bit platform. But here we're using it on explicit 64bit
types (XLogRecPtr).
Now, you could argue that we should make it use 64bit math everywhere -
but I think that might incur quite the price on some 32bit
platforms. It's used in the tuple decoding stuff, that's quite the hot
path in some workloads.
So I guess it's either a separate macro, or we rewrite that piece of
code to work slightly differently and work directly on the lenght or
such.
Maybe we should add a StaticAssert ensuring the TYPEALIGN macro only
gets passed 32bit types?
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
