Jakub Jelinek via Gcc <gcc@gcc.gnu.org> writes: > On Tue, Oct 27, 2020 at 01:18:03PM -0400, Andrew MacLeod via Gcc wrote: >> I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596 >> >> and the ranger is constructing some 128 bit constants and calling >> wide_int_to_tree to turn them into trees. >> >> In particular, it starts with the value >> >> p r.lower_bound(0) >> {<wide_int_storage> = {val = {-65535, 9223372036854775807, 140737488257608}, >> len = 2, precision = 128}, static is_sign_extended = true} >> >> p r.lower_bound(0).dump() >> [0x7fffffffffffffff,0xffffffffffff0001], precision = 128 >> >> >> Â and proceeds to call >> >> wide_int new_lb = wi::set_bit (r.lower_bound (0), 127) >> >> and creates the value: >> >> p new_lb >> {<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128}, >> static is_sign_extended = true} > > This is non-canonical and so invalid, if the low HWI has the MSB set > and the high HWI is -1, it should have been just > val = {-65535}, len = 1, precision = 128} > > I guess the bug is that wi::set_bit_large doesn't call canonize.
Yeah, looks like a micro-optimisation gone wrong. > So perhaps totally untested: > > --- gcc/wide-int.cc.jj 2020-10-19 18:42:41.134426398 +0200 > +++ gcc/wide-int.cc 2020-10-27 18:33:38.546703763 +0100 > @@ -702,8 +702,11 @@ wi::set_bit_large (HOST_WIDE_INT *val, c > /* If the bit we just set is at the msb of the block, make sure > that any higher bits are zeros. */ > if (bit + 1 < precision && subbit == HOST_BITS_PER_WIDE_INT - 1) > - val[len++] = 0; > - return len; > + { > + val[len++] = 0; > + return len; > + } > + return canonize (val, len, precision); > } > else > { LGTM, thanks. Richard