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)