| Message ID | d935df71-7e0d-65fc-0321-d919983641be@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | avoid printing leading 0 in widest_int hex dumps | expand |
Aldy Hernandez <aldyh@redhat.com> writes: > Hi folks! > > Calling print_hex() on a widest_int with the most significant bit turned > on can lead to a leading zero being printed (0x0ffff....). This produces > confusing dumps to say the least, especially when you incorrectly assume > an integer is NOT signed :). That's the intended behaviour though. wide_int-based types only use as many HWIs as they need to store their current value, with any other bits in the value being a sign extension of the top bit. So if the most significant HWI in a widest_int is zero, that HWI is there to say that the previous HWI should be zero- rather than sign-extended. So: 0x0ffffffff -> (1 << 32) - 1 to infinite precision (i.e. a positive value) 0xffffffff -> -1 Thanks, Richard
On Oct 17, 2017, at 5:18 AM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > > Aldy Hernandez <aldyh@redhat.com> writes: >> This produces confusing dumps to say the least > That's the intended behaviour though. > 0x0ffffffff -> (1 << 32) - 1 to infinite precision > (i.e. a positive value) > 0xffffffff -> -1 Another potential way around this would be to print the leading - when applicable and negate the number. I don't have any strong opinions about this, but thought I would mention it. This would then allow the trimming of the leading 0 without confusion.
On 10/17/2017 08:18 AM, Richard Sandiford wrote: > Aldy Hernandez <aldyh@redhat.com> writes: >> Hi folks! >> >> Calling print_hex() on a widest_int with the most significant bit turned >> on can lead to a leading zero being printed (0x0ffff....). This produces >> confusing dumps to say the least, especially when you incorrectly assume >> an integer is NOT signed :). > That's the intended behaviour though. wide_int-based types only use as > many HWIs as they need to store their current value, with any other bits > in the value being a sign extension of the top bit. So if the most > significant HWI in a widest_int is zero, that HWI is there to say that > the previous HWI should be zero- rather than sign-extended. > > So: > > 0x0ffffffff -> (1 << 32) - 1 to infinite precision > (i.e. a positive value) > 0xffffffff -> -1 > > Thanks, > Richard I for one find this very confusing. If I have a 128 bit value, I don't expect to see a 132 bits. And there are enough 0's its not obvious when I look. I don't think a leading 0 should be printed if "precision" bits have already been printed. Andrew
Andrew MacLeod <amacleod@redhat.com> writes: > On 10/17/2017 08:18 AM, Richard Sandiford wrote: >> Aldy Hernandez <aldyh@redhat.com> writes: >>> Hi folks! >>> >>> Calling print_hex() on a widest_int with the most significant bit turned >>> on can lead to a leading zero being printed (0x0ffff....). This produces >>> confusing dumps to say the least, especially when you incorrectly assume >>> an integer is NOT signed :). >> That's the intended behaviour though. wide_int-based types only use as >> many HWIs as they need to store their current value, with any other bits >> in the value being a sign extension of the top bit. So if the most >> significant HWI in a widest_int is zero, that HWI is there to say that >> the previous HWI should be zero- rather than sign-extended. >> >> So: >> >> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >> (i.e. a positive value) >> 0xffffffff -> -1 >> >> Thanks, >> Richard > > I for one find this very confusing. If I have a 128 bit value, I don't > expect to see a 132 bits. And there are enough 0's its not obvious when > I look. But Aldy was talking about widest_int, which is wider than 128 bits. It's an approximation of infinite precision. wide_int is the type to use if you want an N-bit number (for some N). > I don't think a leading 0 should be printed if "precision" bits have > already been printed. Does 0 get printed in that case though? Aldy's patch skips an upper HWI if the upper HWI is zero, but we never have more HWIs than the number that's being represented. Thanks, Richard
On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > Andrew MacLeod <amacleod@redhat.com> writes: >> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>> Aldy Hernandez <aldyh@redhat.com> writes: >>>> Hi folks! >>>> >>>> Calling print_hex() on a widest_int with the most significant bit turned >>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>> confusing dumps to say the least, especially when you incorrectly assume >>>> an integer is NOT signed :). >>> That's the intended behaviour though. wide_int-based types only use as >>> many HWIs as they need to store their current value, with any other bits >>> in the value being a sign extension of the top bit. So if the most >>> significant HWI in a widest_int is zero, that HWI is there to say that >>> the previous HWI should be zero- rather than sign-extended. >>> >>> So: >>> >>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>> (i.e. a positive value) >>> 0xffffffff -> -1 >>> >>> Thanks, >>> Richard >> >> I for one find this very confusing. If I have a 128 bit value, I don't >> expect to see a 132 bits. And there are enough 0's its not obvious when >> I look. > > But Aldy was talking about widest_int, which is wider than 128 bits. > It's an approximation of infinite precision. IMO, we should document this leading zero in print_hex, as it's not inherently obvious. But yes, I was talking about widest_int. I should explain what I am trying to accomplish, since perhaps there is a better way. I am printing a a wide_int (bounds[i] below), but I really don't want to print the sign extension nonsense, since it's a detail of the underlying representation. Currently I'm doing this to chop off the unnecessary bits: /* Wide ints may be sign extended to the full extent of the underlying HWI storage, even if the precision we care about is smaller. Chop off the excess bits for prettier output. */ signop sign = TYPE_UNSIGNED (type) ? UNSIGNED : SIGNED; widest_int val = widest_int::from (bounds[i], sign); val &= wi::mask<widest_int> (bounds[i].get_precision (), false); if (val > 0xffff) print_hex (val, pp_buffer (buffer)->digit_buffer); else print_dec (val, pp_buffer (buffer)->digit_buffer, sign); Since I am calling print_hex() on the widest_int, I get the leading 0. Do you recommend another way of accomplishing this? > > wide_int is the type to use if you want an N-bit number (for some N). > >> I don't think a leading 0 should be printed if "precision" bits have >> already been printed. > > Does 0 get printed in that case though? Aldy's patch skips an upper No. Thanks for your input Richard. Aldy
Aldy Hernandez <aldyh@redhat.com> writes: > On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford > <richard.sandiford@linaro.org> wrote: >> Andrew MacLeod <amacleod@redhat.com> writes: >>> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>> Hi folks! >>>>> >>>>> Calling print_hex() on a widest_int with the most significant bit turned >>>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>>> confusing dumps to say the least, especially when you incorrectly assume >>>>> an integer is NOT signed :). >>>> That's the intended behaviour though. wide_int-based types only use as >>>> many HWIs as they need to store their current value, with any other bits >>>> in the value being a sign extension of the top bit. So if the most >>>> significant HWI in a widest_int is zero, that HWI is there to say that >>>> the previous HWI should be zero- rather than sign-extended. >>>> >>>> So: >>>> >>>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>>> (i.e. a positive value) >>>> 0xffffffff -> -1 >>>> >>>> Thanks, >>>> Richard >>> >>> I for one find this very confusing. If I have a 128 bit value, I don't >>> expect to see a 132 bits. And there are enough 0's its not obvious when >>> I look. >> >> But Aldy was talking about widest_int, which is wider than 128 bits. >> It's an approximation of infinite precision. > > IMO, we should document this leading zero in print_hex, as it's not > inherently obvious. > > But yes, I was talking about widest_int. I should explain what I am > trying to accomplish, since perhaps there is a better way. > > I am printing a a wide_int (bounds[i] below), but I really don't want > to print the sign extension nonsense, since it's a detail of the > underlying representation. Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension bits above the precision. I think it'd work if print_hex used extract_uhwi insteead of elt, which would also remove the need to handle "negative" numbers specially. I'll try that tomorrow. > Currently I'm doing this to chop off the unnecessary bits: > > /* Wide ints may be sign extended to the full extent of the > underlying HWI storage, even if the precision we care about > is smaller. Chop off the excess bits for prettier output. */ > signop sign = TYPE_UNSIGNED (type) ? UNSIGNED : SIGNED; > widest_int val = widest_int::from (bounds[i], sign); > val &= wi::mask<widest_int> (bounds[i].get_precision (), false); > > if (val > 0xffff) > print_hex (val, pp_buffer (buffer)->digit_buffer); > else > print_dec (val, pp_buffer (buffer)->digit_buffer, sign); > > Since I am calling print_hex() on the widest_int, I get the leading 0. > > Do you recommend another way of accomplishing this? Is it just print_hex that's the problem? Does print_dec handle big negative numbers properly? I agree we should make it so that wide-int-print.cc does the right thing for this without the need to switch to widest_int. Thanks, Richard > >> >> wide_int is the type to use if you want an N-bit number (for some N). >> >>> I don't think a leading 0 should be printed if "precision" bits have >>> already been printed. >> >> Does 0 get printed in that case though? Aldy's patch skips an upper > > No. > > Thanks for your input Richard. > Aldy
On 10/18/2017 06:39 PM, Richard Sandiford wrote: > Aldy Hernandez <aldyh@redhat.com> writes: >> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford > Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension > bits above the precision. I think it'd work if print_hex used > extract_uhwi insteead of elt, which would also remove the need > to handle "negative" numbers specially. I'll try that tomorrow. Thanks. > >> Currently I'm doing this to chop off the unnecessary bits: >> >> /* Wide ints may be sign extended to the full extent of the >> underlying HWI storage, even if the precision we care about >> is smaller. Chop off the excess bits for prettier output. */ >> signop sign = TYPE_UNSIGNED (type) ? UNSIGNED : SIGNED; >> widest_int val = widest_int::from (bounds[i], sign); >> val &= wi::mask<widest_int> (bounds[i].get_precision (), false); >> >> if (val > 0xffff) >> print_hex (val, pp_buffer (buffer)->digit_buffer); >> else >> print_dec (val, pp_buffer (buffer)->digit_buffer, sign); >> >> Since I am calling print_hex() on the widest_int, I get the leading 0. >> >> Do you recommend another way of accomplishing this? > > Is it just print_hex that's the problem? Does print_dec handle > big negative numbers properly? I haven't checked. I don't bother printing in decimal when the number is larger than say, 0xffff, hence the code above :). Aldy
Richard Sandiford <richard.sandiford@linaro.org> writes: > Aldy Hernandez <aldyh@redhat.com> writes: >> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford >> <richard.sandiford@linaro.org> wrote: >>> Andrew MacLeod <amacleod@redhat.com> writes: >>>> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>>> Hi folks! >>>>>> >>>>>> Calling print_hex() on a widest_int with the most significant bit turned >>>>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>>>> confusing dumps to say the least, especially when you incorrectly assume >>>>>> an integer is NOT signed :). >>>>> That's the intended behaviour though. wide_int-based types only use as >>>>> many HWIs as they need to store their current value, with any other bits >>>>> in the value being a sign extension of the top bit. So if the most >>>>> significant HWI in a widest_int is zero, that HWI is there to say that >>>>> the previous HWI should be zero- rather than sign-extended. >>>>> >>>>> So: >>>>> >>>>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>>>> (i.e. a positive value) >>>>> 0xffffffff -> -1 >>>>> >>>>> Thanks, >>>>> Richard >>>> >>>> I for one find this very confusing. If I have a 128 bit value, I don't >>>> expect to see a 132 bits. And there are enough 0's its not obvious when >>>> I look. >>> >>> But Aldy was talking about widest_int, which is wider than 128 bits. >>> It's an approximation of infinite precision. >> >> IMO, we should document this leading zero in print_hex, as it's not >> inherently obvious. >> >> But yes, I was talking about widest_int. I should explain what I am >> trying to accomplish, since perhaps there is a better way. >> >> I am printing a a wide_int (bounds[i] below), but I really don't want >> to print the sign extension nonsense, since it's a detail of the >> underlying representation. > > Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension > bits above the precision. I think it'd work if print_hex used > extract_uhwi insteead of elt, which would also remove the need > to handle "negative" numbers specially. I'll try that tomorrow. How about this? Not tested much beyond the selftests themselves. Thanks, Richard gcc/ * wide-int-print.cc (print_hex): Loop based on extract_uhwi. Don't print any bits outside the precision of the value. * wide-int.cc (test_printing): Add some new tests. Index: gcc/wide-int-print.cc =================================================================== --- gcc/wide-int-print.cc 2017-07-02 10:05:20.997439436 +0100 +++ gcc/wide-int-print.cc 2017-10-19 21:20:05.138500726 +0100 @@ -103,30 +103,28 @@ print_decu (const wide_int_ref &wi, FILE } void -print_hex (const wide_int_ref &wi, char *buf) +print_hex (const wide_int_ref &val, char *buf) { - int i = wi.get_len (); - - if (wi == 0) + if (val == 0) buf += sprintf (buf, "0x0"); else { - if (wi::neg_p (wi)) + buf += sprintf (buf, "0x"); + int start = ROUND_DOWN (val.get_precision (), HOST_BITS_PER_WIDE_INT); + int width = val.get_precision () - start; + bool first_p = true; + for (int i = start; i >= 0; i -= HOST_BITS_PER_WIDE_INT) { - int j; - /* If the number is negative, we may need to pad value with - 0xFFF... because the leading elements may be missing and - we do not print a '-' with hex. */ - buf += sprintf (buf, "0x"); - for (j = BLOCKS_NEEDED (wi.get_precision ()); j > i; j--) - buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, HOST_WIDE_INT_M1); - + unsigned HOST_WIDE_INT uhwi = wi::extract_uhwi (val, i, width); + if (!first_p) + buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, uhwi); + else if (uhwi != 0) + { + buf += sprintf (buf, HOST_WIDE_INT_PRINT_HEX_PURE, uhwi); + first_p = false; + } + width = HOST_BITS_PER_WIDE_INT; } - else - buf += sprintf (buf, "0x" HOST_WIDE_INT_PRINT_HEX_PURE, wi.elt (--i)); - - while (--i >= 0) - buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, wi.elt (i)); } } Index: gcc/wide-int.cc =================================================================== --- gcc/wide-int.cc 2017-10-19 21:19:47.100454414 +0100 +++ gcc/wide-int.cc 2017-10-19 21:20:05.138500726 +0100 @@ -2253,6 +2253,17 @@ test_printing () VALUE_TYPE a = from_int<VALUE_TYPE> (42); assert_deceq ("42", a, SIGNED); assert_hexeq ("0x2a", a); + assert_hexeq ("0x1fffffffffffffffff", wi::shwi (-1, 69)); + assert_hexeq ("0xffffffffffffffff", wi::mask (64, false, 69)); + assert_hexeq ("0xffffffffffffffff", wi::mask <widest_int> (64, false)); + if (WIDE_INT_MAX_PRECISION > 128) + { + assert_hexeq ("0x20000000000000000fffffffffffffffe", + wi::lshift (1, 129) + wi::lshift (1, 64) - 2); + assert_hexeq ("0x200000000000004000123456789abcdef", + wi::lshift (1, 129) + wi::lshift (1, 74) + + wi::lshift (0x1234567, 32) + 0x89abcdef); + } } /* Verify that various operations work correctly for VALUE_TYPE,
On 10/19/2017 04:22 PM, Richard Sandiford wrote: > Richard Sandiford <richard.sandiford@linaro.org> writes: >> Aldy Hernandez <aldyh@redhat.com> writes: >>> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford >>> <richard.sandiford@linaro.org> wrote: >>>> Andrew MacLeod <amacleod@redhat.com> writes: >>>>> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>>>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>>>> Hi folks! >>>>>>> >>>>>>> Calling print_hex() on a widest_int with the most significant bit turned >>>>>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>>>>> confusing dumps to say the least, especially when you incorrectly assume >>>>>>> an integer is NOT signed :). >>>>>> That's the intended behaviour though. wide_int-based types only use as >>>>>> many HWIs as they need to store their current value, with any other bits >>>>>> in the value being a sign extension of the top bit. So if the most >>>>>> significant HWI in a widest_int is zero, that HWI is there to say that >>>>>> the previous HWI should be zero- rather than sign-extended. >>>>>> >>>>>> So: >>>>>> >>>>>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>>>>> (i.e. a positive value) >>>>>> 0xffffffff -> -1 >>>>>> >>>>>> Thanks, >>>>>> Richard >>>>> I for one find this very confusing. If I have a 128 bit value, I don't >>>>> expect to see a 132 bits. And there are enough 0's its not obvious when >>>>> I look. >>>> But Aldy was talking about widest_int, which is wider than 128 bits. >>>> It's an approximation of infinite precision. >>> IMO, we should document this leading zero in print_hex, as it's not >>> inherently obvious. >>> >>> But yes, I was talking about widest_int. I should explain what I am >>> trying to accomplish, since perhaps there is a better way. >>> >>> I am printing a a wide_int (bounds[i] below), but I really don't want >>> to print the sign extension nonsense, since it's a detail of the >>> underlying representation. >> Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension >> bits above the precision. I think it'd work if print_hex used >> extract_uhwi insteead of elt, which would also remove the need >> to handle "negative" numbers specially. I'll try that tomorrow. > How about this? Not tested much beyond the selftests themselves. > > Thanks, > Richard > > > gcc/ > * wide-int-print.cc (print_hex): Loop based on extract_uhwi. > Don't print any bits outside the precision of the value. > * wide-int.cc (test_printing): Add some new tests. > This does seem to resolve my printing issues. THanks Andrew
Andrew MacLeod <amacleod@redhat.com> writes: > On 10/19/2017 04:22 PM, Richard Sandiford wrote: >> Richard Sandiford <richard.sandiford@linaro.org> writes: >>> Aldy Hernandez <aldyh@redhat.com> writes: >>>> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford >>>> <richard.sandiford@linaro.org> wrote: >>>>> Andrew MacLeod <amacleod@redhat.com> writes: >>>>>> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>>>>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>>>>> Hi folks! >>>>>>>> >>>>>>>> Calling print_hex() on a widest_int with the most significant bit turned >>>>>>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>>>>>> confusing dumps to say the least, especially when you incorrectly assume >>>>>>>> an integer is NOT signed :). >>>>>>> That's the intended behaviour though. wide_int-based types only use as >>>>>>> many HWIs as they need to store their current value, with any other bits >>>>>>> in the value being a sign extension of the top bit. So if the most >>>>>>> significant HWI in a widest_int is zero, that HWI is there to say that >>>>>>> the previous HWI should be zero- rather than sign-extended. >>>>>>> >>>>>>> So: >>>>>>> >>>>>>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>>>>>> (i.e. a positive value) >>>>>>> 0xffffffff -> -1 >>>>>>> >>>>>>> Thanks, >>>>>>> Richard >>>>>> I for one find this very confusing. If I have a 128 bit value, I don't >>>>>> expect to see a 132 bits. And there are enough 0's its not obvious when >>>>>> I look. >>>>> But Aldy was talking about widest_int, which is wider than 128 bits. >>>>> It's an approximation of infinite precision. >>>> IMO, we should document this leading zero in print_hex, as it's not >>>> inherently obvious. >>>> >>>> But yes, I was talking about widest_int. I should explain what I am >>>> trying to accomplish, since perhaps there is a better way. >>>> >>>> I am printing a a wide_int (bounds[i] below), but I really don't want >>>> to print the sign extension nonsense, since it's a detail of the >>>> underlying representation. >>> Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension >>> bits above the precision. I think it'd work if print_hex used >>> extract_uhwi insteead of elt, which would also remove the need >>> to handle "negative" numbers specially. I'll try that tomorrow. >> How about this? Not tested much beyond the selftests themselves. >> >> Thanks, >> Richard >> >> >> gcc/ >> * wide-int-print.cc (print_hex): Loop based on extract_uhwi. >> Don't print any bits outside the precision of the value. >> * wide-int.cc (test_printing): Add some new tests. >> > This does seem to resolve my printing issues. Thanks. Now tested on aarch64-linux-gnu, powerpc64le-linux-gnu and x86_64-linux-gnu. OK to install? Richard gcc/ * wide-int-print.cc (print_hex): Loop based on extract_uhwi. Don't print any bits outside the precision of the value. * wide-int.cc (test_printing): Add some new tests. Index: gcc/wide-int-print.cc =================================================================== --- gcc/wide-int-print.cc 2017-07-02 10:05:20.997439436 +0100 +++ gcc/wide-int-print.cc 2017-10-19 21:20:05.138500726 +0100 @@ -103,30 +103,28 @@ print_decu (const wide_int_ref &wi, FILE } void -print_hex (const wide_int_ref &wi, char *buf) +print_hex (const wide_int_ref &val, char *buf) { - int i = wi.get_len (); - - if (wi == 0) + if (val == 0) buf += sprintf (buf, "0x0"); else { - if (wi::neg_p (wi)) + buf += sprintf (buf, "0x"); + int start = ROUND_DOWN (val.get_precision (), HOST_BITS_PER_WIDE_INT); + int width = val.get_precision () - start; + bool first_p = true; + for (int i = start; i >= 0; i -= HOST_BITS_PER_WIDE_INT) { - int j; - /* If the number is negative, we may need to pad value with - 0xFFF... because the leading elements may be missing and - we do not print a '-' with hex. */ - buf += sprintf (buf, "0x"); - for (j = BLOCKS_NEEDED (wi.get_precision ()); j > i; j--) - buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, HOST_WIDE_INT_M1); - + unsigned HOST_WIDE_INT uhwi = wi::extract_uhwi (val, i, width); + if (!first_p) + buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, uhwi); + else if (uhwi != 0) + { + buf += sprintf (buf, HOST_WIDE_INT_PRINT_HEX_PURE, uhwi); + first_p = false; + } + width = HOST_BITS_PER_WIDE_INT; } - else - buf += sprintf (buf, "0x" HOST_WIDE_INT_PRINT_HEX_PURE, wi.elt (--i)); - - while (--i >= 0) - buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, wi.elt (i)); } } Index: gcc/wide-int.cc =================================================================== --- gcc/wide-int.cc 2017-10-19 21:19:47.100454414 +0100 +++ gcc/wide-int.cc 2017-10-19 21:20:05.138500726 +0100 @@ -2253,6 +2253,17 @@ test_printing () VALUE_TYPE a = from_int<VALUE_TYPE> (42); assert_deceq ("42", a, SIGNED); assert_hexeq ("0x2a", a); + assert_hexeq ("0x1fffffffffffffffff", wi::shwi (-1, 69)); + assert_hexeq ("0xffffffffffffffff", wi::mask (64, false, 69)); + assert_hexeq ("0xffffffffffffffff", wi::mask <widest_int> (64, false)); + if (WIDE_INT_MAX_PRECISION > 128) + { + assert_hexeq ("0x20000000000000000fffffffffffffffe", + wi::lshift (1, 129) + wi::lshift (1, 64) - 2); + assert_hexeq ("0x200000000000004000123456789abcdef", + wi::lshift (1, 129) + wi::lshift (1, 74) + + wi::lshift (0x1234567, 32) + 0x89abcdef); + } } /* Verify that various operations work correctly for VALUE_TYPE,
On 10/24/2017 10:24 AM, Richard Sandiford wrote: > Andrew MacLeod <amacleod@redhat.com> writes: >> On 10/19/2017 04:22 PM, Richard Sandiford wrote: >>> Richard Sandiford <richard.sandiford@linaro.org> writes: >>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford >>>>> <richard.sandiford@linaro.org> wrote: >>>>>> Andrew MacLeod <amacleod@redhat.com> writes: >>>>>>> On 10/17/2017 08:18 AM, Richard Sandiford wrote: >>>>>>>> Aldy Hernandez <aldyh@redhat.com> writes: >>>>>>>>> Hi folks! >>>>>>>>> >>>>>>>>> Calling print_hex() on a widest_int with the most significant bit turned >>>>>>>>> on can lead to a leading zero being printed (0x0ffff....). This produces >>>>>>>>> confusing dumps to say the least, especially when you incorrectly assume >>>>>>>>> an integer is NOT signed :). >>>>>>>> That's the intended behaviour though. wide_int-based types only use as >>>>>>>> many HWIs as they need to store their current value, with any other bits >>>>>>>> in the value being a sign extension of the top bit. So if the most >>>>>>>> significant HWI in a widest_int is zero, that HWI is there to say that >>>>>>>> the previous HWI should be zero- rather than sign-extended. >>>>>>>> >>>>>>>> So: >>>>>>>> >>>>>>>> 0x0ffffffff -> (1 << 32) - 1 to infinite precision >>>>>>>> (i.e. a positive value) >>>>>>>> 0xffffffff -> -1 >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Richard >>>>>>> I for one find this very confusing. If I have a 128 bit value, I don't >>>>>>> expect to see a 132 bits. And there are enough 0's its not obvious when >>>>>>> I look. >>>>>> But Aldy was talking about widest_int, which is wider than 128 bits. >>>>>> It's an approximation of infinite precision. >>>>> IMO, we should document this leading zero in print_hex, as it's not >>>>> inherently obvious. >>>>> >>>>> But yes, I was talking about widest_int. I should explain what I am >>>>> trying to accomplish, since perhaps there is a better way. >>>>> >>>>> I am printing a a wide_int (bounds[i] below), but I really don't want >>>>> to print the sign extension nonsense, since it's a detail of the >>>>> underlying representation. >>>> Ah! OK. Yeah, I agree it doesn't make sense to print sign-extension >>>> bits above the precision. I think it'd work if print_hex used >>>> extract_uhwi insteead of elt, which would also remove the need >>>> to handle "negative" numbers specially. I'll try that tomorrow. >>> How about this? Not tested much beyond the selftests themselves. >>> >>> Thanks, >>> Richard >>> >>> >>> gcc/ >>> * wide-int-print.cc (print_hex): Loop based on extract_uhwi. >>> Don't print any bits outside the precision of the value. >>> * wide-int.cc (test_printing): Add some new tests. >>> >> This does seem to resolve my printing issues. > > Thanks. Now tested on aarch64-linux-gnu, powerpc64le-linux-gnu and > x86_64-linux-gnu. OK to install? > > Richard > > > gcc/ > * wide-int-print.cc (print_hex): Loop based on extract_uhwi. > Don't print any bits outside the precision of the value. > * wide-int.cc (test_printing): Add some new tests. OK. Jeff
On 10/24/2017 12:24 PM, Richard Sandiford wrote: > Andrew MacLeod <amacleod@redhat.com> writes: >> On 10/19/2017 04:22 PM, Richard Sandiford wrote: >>> Richard Sandiford <richard.sandiford@linaro.org> writes: >>>> Aldy Hernandez <aldyh@redhat.com> writes: >>> gcc/ >>> * wide-int-print.cc (print_hex): Loop based on extract_uhwi. >>> Don't print any bits outside the precision of the value. >>> * wide-int.cc (test_printing): Add some new tests. >>> >> This does seem to resolve my printing issues. > > Thanks. Now tested on aarch64-linux-gnu, powerpc64le-linux-gnu and > x86_64-linux-gnu. OK to install? > > Richard Thank you very much Richard. I've incorporated this into our branch. Aldy
gcc/ * wide-int-print.cc (print_hex): Avoid printing a leading zero. * wide-int.cc (test_printing): Add test for hex dumping of a widest_int conversion. diff --git a/gcc/wide-int-print.cc b/gcc/wide-int-print.cc index 36d8ad863f5..36a681d4b73 100644 --- a/gcc/wide-int-print.cc +++ b/gcc/wide-int-print.cc @@ -123,7 +123,13 @@ print_hex (const wide_int_ref &wi, char *buf) } else - buf += sprintf (buf, "0x" HOST_WIDE_INT_PRINT_HEX_PURE, wi.elt (--i)); + { + buf += sprintf (buf, "0x"); + --i; + /* Avoid printing a leading zero. */ + if (wi.elt (i)) + buf += sprintf (buf, HOST_WIDE_INT_PRINT_HEX_PURE, wi.elt (i)); + } while (--i >= 0) buf += sprintf (buf, HOST_WIDE_INT_PRINT_PADDED_HEX, wi.elt (i)); diff --git a/gcc/wide-int.cc b/gcc/wide-int.cc index 71e24ec22af..123a70fa9b8 100644 --- a/gcc/wide-int.cc +++ b/gcc/wide-int.cc @@ -2220,6 +2220,14 @@ test_printing () VALUE_TYPE a = from_int<VALUE_TYPE> (42); assert_deceq ("42", a, SIGNED); assert_hexeq ("0x2a", a); + + /* Test that converting to widest_int still produces a sane hex + representation. */ + VALUE_TYPE big = from_int<VALUE_TYPE> (0xffffffff); + widest_int huge = widest_int::from (big, UNSIGNED); + char buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_hex (huge, buf); + ASSERT_TRUE (buf[0] == '0' && buf[1] == 'x' && buf[2] == 'f'); } /* Verify that various operations work correctly for VALUE_TYPE,