Saturated Add and Mul in x86 segfaults

[Nordlöw]

Hey there!

The following code segfaults on my Ubuntu Desktop. Why?

static inline __attribute__ ((pure)) uint32_t uint32_sadd(uint32_t a,
uint32_t b)
{
#if defined (__GNUC__) && defined (__i386__)
uint32_t c;
__asm__ volatile ("movl %1, %%eax \n\t"
"addl %2, %%eax \n\t"
"movl 0xffffffff, %%ecx \n\t"
"cmovc %%ecx, %%eax \n\t"
"movl %%eax, %0 \n\t"
: "=r" (c)
: "r" (a), "r" (b));
return c;
#else
return (UINT32_MAX - a < b) ? UINT32_MAX : a + b;
#endif
}

Thanks in advance,
Nordlöw

[Frank Kotler]

Nordlöw wrote:

....

> "movl 0xffffffff, %%ecx \n\t"

I'm not certain about "inline" syntax, but in "plain Gas" that tries to
load the contents of memory at 0xffffffff into ecx... which would
account for the segfault :) Try "$0xffffffff".

Best,
Frank

[Sebastian Biallas]

Nordlöw wrote:
> static inline __attribute__ ((pure)) uint32_t uint32_sadd(uint32_t a,
> uint32_t b)
> {
> #if defined (__GNUC__) && defined (__i386__)
> uint32_t c;
> __asm__ volatile ("movl %1, %%eax \n\t"
> "addl %2, %%eax \n\t"
> "movl 0xffffffff, %%ecx \n\t"
> "cmovc %%ecx, %%eax \n\t"
> "movl %%eax, %0 \n\t"
> : "=r" (c)
> : "r" (a), "r" (b));
> return c;
> #else
> return (UINT32_MAX - a < b) ? UINT32_MAX : a + b;
> #endif
> }

Replace it with:

uint32_t uint32_sadd(uint32_t a, uint32_t b)
{

return (a + b < b) ? UINT32_MAX : a + b;
}

gcc-4.3.2 compiles this into:
uint32_sadd:
movl 8(%esp), %eax
addl 4(%esp), %eax
sbbl %edx, %edx
orl %edx, %eax
ret

Which is (I guess) even better than your code :)

[Timothy Baldwin]

In message
<25858373-c13c-47d7...@l33g2000pri.googlegroups.com>,
Nordlöw <spam...@crayne.org> wrote:

> static inline __attribute__ ((pure)) uint32_t uint32_sadd(uint32_t a,
> uint32_t b)
> {
> #if defined (__GNUC__) && defined (__i386__)

You are using GCC specific features outside your test for GCC, and cmov is
not available in 386 processors so your condition is wrong. __attribute__
((pure)) is redundant on an inline function.

>   uint32_t c;
> __asm__ volatile ("movl %1, %%eax \n\t"
> "addl %2, %%eax \n\t"
> "movl 0xffffffff, %%ecx \n\t"
> "cmovc %%ecx, %%eax \n\t"
> "movl %%eax, %0 \n\t"
> : "=r" (c)
> : "r" (a), "r" (b));

You are changing eax and ecx without declaring them as outputs. Also your
explicit use of registers is non-optimal, the compiler may be able to
provide a better choice.

Lose the volatile as well, as this inline assembler has no side effects. It
is also possible to optimise for constants:

static inline __attribute__ ((pure)) uint32_t uint32_sadd(uint32_t a,
uint32_t b)
{

uint32_t c = (UINT32_MAX - a < b) ? UINT32_MAX : a + b;

#if defined (__GNUC__) && defined (__i386__)

if (!__builtin_constant_p(c)) {
__asm__ ("addl %2, %0 \n\t"
"cmovc %3, %0 \n\t"
: "=&r" (c)
: "%0" (a), "g" (b), "r" (0xFFFFFFFF));
}
#endif
return c;
}

Consider using MMX or SSE.