Peter Eisentraut <pe...@eisentraut.org> writes:
> On 09.06.25 10:21, Michael Paquier wrote:
>> now in detoast.h:
>> /*
>> * Macro to fetch the possibly-unaligned contents of an EXTERNAL datum
>> * into a local "struct varatt_external" toast pointer. This should be
>> * just a memcpy, but some versions of gcc seem to produce broken code
>> * that assumes the datum contents are aligned. Introducing an explicit
>> * intermediate "varattrib_1b_e *" variable seems to fix it.
>> */
> I'm not sure that the original reason applies anymore.
I don't see why it wouldn't; if anything, compilers have gotten more
eager to optimize on the strength of dubious assumptions. Perhaps
we are less likely to notice now that there are so few machines that
will throw a bus error for misaligned accesses --- but I believe that
those are still quite expensive on a lot of platforms, so we'd do
well to avoid them.
> If attr in the above code is of type Datum, then I think the original
> problem still exists. The compiler can assume that values of type Datum
> have alignment fitting for Datum.
I think this argument is confusing the alignment of the pointer datum
with that of the struct to which it points.
> I can see how this might have been different historically. I have
> noticed that there are some areas of code where Datum and struct varlena
> * or similar are used interchangeably. Macros tend to hide that kind of
> confusion. But some of this has been cleaned up with changing some
> macros to inline functions. Maybe doing the same would help here, too.
I don't have any particular beef with changing this code to use
an inline function, if we can figure out how to make the sizeof()
bits work. Right now it's agnostic about how big toast_pointer is,
which seems like a good property to preserve if we are thinking
about having multiple kinds of toast pointer.
regards, tom lane
