Andrew MacLeod <amacl...@redhat.com> writes:
> On 10/19/2017 04:22 PM, Richard Sandiford wrote:
>> Richard Sandiford <richard.sandif...@linaro.org> writes:
>>> Aldy Hernandez <al...@redhat.com> writes:
>>>> On Tue, Oct 17, 2017 at 6:05 PM, Richard Sandiford
>>>> <richard.sandif...@linaro.org> wrote:
>>>>> Andrew MacLeod <amacl...@redhat.com> writes:
>>>>>> On 10/17/2017 08:18 AM, Richard Sandiford wrote:
>>>>>>> Aldy Hernandez <al...@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,
