[patch] avoid printing leading 0 in widest_int hex dumps

[patch] avoid printing leading 0 in widest_int hex dumps

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

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 :).

OK for trunk?

[-- Attachment #2: curr.patch --]
[-- Type: text/x-patch, Size: 1431 bytes --]

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,

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Richard Sandiford]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Mike Stump]

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.

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Andrew MacLeod]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Richard Sandiford]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Aldy Hernandez]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Richard Sandiford]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Aldy Hernandez]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Richard Sandiford]

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,

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Andrew MacLeod]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Richard Sandiford]

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,

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Jeff Law]

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

Re: [patch] avoid printing leading 0 in widest_int hex dumps

[Aldy Hernandez]

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