On 2018-07-09 17:08:34 -0700, Andres Freund wrote:
> Hi,
>
> On 2018-07-09 19:56:25 -0400, Tom Lane wrote:
> > Andres Freund <and...@anarazel.de> writes:
> > > On 2018-07-10 11:35:59 +1200, Thomas Munro wrote:
> > >> I think it's probably a good idea to make it very explicit when moving
> > >> between big and small transaction IDs, hence the including of the word
> > >> 'big' in variable and function names and the use of a function-like
> > >> macro (rather than implicit conversion, which C doesn't give me a good
> > >> way to prevent). Otherwise there is a class of bug that is hidden for
> > >> the first 2^32 transactions.
> >
> > > You could have BigTransactionId (maybe renamed to FullTransactionId?) be
> > > a struct type. That'd prevent such issues. Most compilers these days
> > > should be more than good enough to optimize passing around an 8byte
> > > struct by value...
> >
> > Or, perhaps, use a struct in assert builds and int64 otherwise?
> > You could hide the ensuing notational differences in macros.
>
> That should be doable. But I'd like to check if it's necessary
> first. Optimizing passing an 8 byte struct shouldn't be hard for
> compilers these days - especially when most things dealing with them are
> inline functions. If we find that it's not a problem on contemporary
> compilers, it might be worthwhile to use a bit more type safety in other
> places too.
>
> Here's what gcc does on O1:
>
> #include <stdint.h>
>
> typedef struct foo
> {
> uint64_t id;
> } foo;
>
> extern foo take_foo_struct(foo f, int i);
> extern uint64_t take_foo_int(uint64_t id, int i);
>
> foo take_foo_struct(foo f, int i)
> {
> f.id += i;
> return f;
> }
>
> uint64_t take_foo_int(uint64_t id, int i)
> {
> id += i;
> return id;
> }
>
> results in:
>
> .file "test.c"
> .text
> .globl take_foo_struct
> .type take_foo_struct, @function
> take_foo_struct:
> .LFB0:
> .cfi_startproc
> movslq %esi, %rax
> addq %rdi, %rax
> ret
> .cfi_endproc
> .LFE0:
> .size take_foo_struct, .-take_foo_struct
> .globl take_foo_int
> .type take_foo_int, @function
> take_foo_int:
> .LFB1:
> .cfi_startproc
> movslq %esi, %rax
> addq %rdi, %rax
> ret
> .cfi_endproc
> .LFE1:
> .size take_foo_int, .-take_foo_int
> .ident "GCC: (Debian 7.3.0-24) 7.3.0"
> .section .note.GNU-stack,"",@progbits
>
> IOW, exactly the same code generated. Note that the compiler does *not*
> see the callsites in this case, i.e. this is platform ABI conformant.
FWIW, this is required by the x86-64 SYSV ABI. See
https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf
3.2.3 Parameter Passing. Aggregates with scalar types up to "two
eightbytes" are passed via registers.
It's also the case for MSVC / windows
https://docs.microsoft.com/en-us/cpp/cpp/argument-passing-and-naming-conventions
https://docs.microsoft.com/en-us/cpp/build/parameter-passing
Small aggregates (8, 16, 32, or 64 bits) are passed in registers.
Greetings,
Andres Freund
