On Tue, 13 Feb 2024, Jakub Jelinek wrote:
> Hi!
>
> As I wrote earlier, I was seeing
> FAIL: gcc.dg/torture/bitint-24.c -O0 execution test
> FAIL: gcc.dg/torture/bitint-24.c -O2 execution test
> with the ia32 _BitInt enablement patch on i686-linux. I thought
> floatbitintxf.c was miscompiled with -O2 -march=i686 -mtune=generic, but it
> turned out to be UB in it.
>
> If a signed _BitInt to be converted to binary floating point has
> (after sign extension from possible partial limb to full limb) one or
> more most significant limbs equal to all ones and then in the limb below
> (the most significant non-~(UBILtype)0 limb) has the most significant limb
> cleared, like for 32-bit limbs
> 0x81582c05U, 0x0a8b01e4U, 0xc1b8b18fU, 0x2aac2a08U, -1U, -1U
> then bitint_reduce_prec can't reduce it to that 0x2aac2a08U limb, so
> msb is all ones and precision is negative (so it reduced precision from
> 161 to 192 bits down to 160 bits, in theory could go as low as 129 bits
> but that wouldn't change anything on the following behavior).
> But still iprec is negative, -160 here.
> For that case (i.e. where we are dealing with an negative input), the
> code was using 65 - __builtin_clzll (~msb) to compute how many relevant
> bits we have from the msb. Unfortunately that invokes UB for msb all ones.
> The right number of relevant bits in that case is 1 though (like for
> -2 it is 2 and -4 or -3 3 as already computed) - all we care about from that
> is that the most significant bit is set (i.e. the number is negative) and
> the bits below that should be supplied from the limbs below.
>
> So, the following patch fixes it by special casing it not to invoke UB.
>
> For msb 0 we already have a special case from before (but that is also
> different because msb 0 implies the whole number is 0 given the way
> bitint_reduce_prec works - even if we have limbs like ..., 0x80000000U, 0U
> the reduction can skip the most significant limb and msb then would be
> the one below it), so if iprec > 0, we already don't call __builtin_clzll
> on 0.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2024-02-13 Jakub Jelinek <ja...@redhat.com>
>
> * soft-fp/bitint.h (FP_FROM_BITINT): If iprec < 0 and msb is all ones,
> just set n to 1 instead of using __builtin_clzll (~msb).
>
> --- libgcc/soft-fp/bitint.h.jj 2024-01-12 22:06:06.248661862 +0100
> +++ libgcc/soft-fp/bitint.h 2024-02-12 18:56:42.947974875 +0100
> @@ -276,7 +276,11 @@ bitint_negate (UBILtype *d, const UBILty
> } \
> if (iprec < 0) \
> { \
> - n = sizeof (0ULL) * __CHAR_BIT__ + 1 - __builtin_clzll (~msb);\
> + if (msb == (UBILtype) -1) \
> + n = 1; \
> + else \
> + n = (sizeof (0ULL) * __CHAR_BIT__ + 1 \
> + - __builtin_clzll (~msb)); \
> if (BIL_TYPE_SIZE > DI##_BITS && n > DI##_BITS) \
> { \
> iv = msb >> (n - DI##_BITS); \
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
