"warning: assuming signed overflow does not occur" in _adds/_subs emulation

[yumkam]

On debian/stretch:i386 using distro-provided cross-compiler (g++-6-arm-linux-gnueabihf (6.3.0-18cross1))

In file included from $path/userland/interface/khronos/common/khrn_int_util.c:30:0:
$path/userland/interface/khronos/common/khrn_int_util.c: In function 'khrn_clip_range2':
$path/userland/interface/khronos/common/khrn_int_util.h:145:57: warning: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Wstrict-overflow]
    return (y > 0) ? ((z < x) ? (int32_t)0x7fffffff : z) : ((z > x) ? (int32_t)0x80000000 : z);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$path/userland/interface/khronos/common/khrn_int_util.h:151:57: warning: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Wstrict-overflow]
    return (y > 0) ? ((z > x) ? (int32_t)0x80000000 : z) : ((z < x) ? (int32_t)0x7fffffff : z);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$path/userland/interface/khronos/common/khrn_int_util.h:145:57: warning: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Wstrict-overflow]
    return (y > 0) ? ((z < x) ? (int32_t)0x7fffffff : z) : ((z > x) ? (int32_t)0x80000000 : z);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$path/userland/interface/khronos/common/khrn_int_util.h:151:57: warning: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Wstrict-overflow]
    return (y > 0) ? ((z > x) ? (int32_t)0x80000000 : z) : ((z < x) ? (int32_t)0x7fffffff : z);
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Looks like 1) warning is correct: code relies on undefined behavior; 2) despite warning, gcc (as of 6.3.0) generates correct code (that is, it does not actually assume that signed overflow does not occur), but this is fragile and can break anytime (I've also checked raspbian-provided libEGL_static.a, affected generated asm code looks correct).

[6by9]

If we really care, then they can be amended to

static INLINE int32_t _adds(int32_t x, int32_t y)
{
   return (y > 0) ? (((INT_MAX - y) < x) ? INT_MAX : x+y) : (((INT_MIN - y) > x) ? INT_MIN : x+y);
}

static INLINE int32_t _subs(int32_t x, int32_t y)
{
   return (y > 0) ? (((INT_MIN + y) > x) ? INT_MIN : x-y) : (((INT_MAX + y) < x) ? INT_MAX : x-y);
}

(I think I've verified all combinations of saturation). Obviously they're more complex computationally, so performance may be affected.

This is part of the firmware based GLES driver which will hopefully be deprecated sometime soon in favour of the kernel vc4 driver and mesa. The fact that it gives the correct numbers on the assumption that it overflows in the normal manner (ie without signed overflow) makes me not overly fussed by the warning. I don't know what others think.