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.

Greetings,

Andres Freund

Reply via email to