Patchwork Power/AltiVec question (Re: [RFA 0/5] improve printing of 128 bit ints)

login
register
mail settings
Submitter Tom Tromey
Date June 12, 2017, 2:34 p.m.
Message ID <87h8zl46mp.fsf@pokyo>
Download mbox | patch
Permalink /patch/20946/
State New
Headers show

Comments

Tom Tromey - June 12, 2017, 2:34 p.m.
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I'd expect that it prints 0x0 because "0x00000001", when reinterpreted (note,
Pedro> not cast/converted) as the storage for a 32-bit float of whatever
Pedro> format AltiVec uses, gives you a real number between 0 and 1.  And
Pedro> then, considering <https://sourceware.org/bugzilla/show_bug.cgi?id=15318>,
Pedro> when that number is converted to integer for printing as hex, it
Pedro> it is converted to zero [as in, (int)0.1 => 0].

That doesn't seem too useful to me, but I've managed to preserve the behavior.

Here's an updated version of patch #2 from the series.
With this the series tests cleanly on the buildbot.

Tom

commit baa4f3cf4dae51541dcec33c8733aa2d5ee9e7c1
Author: Tom Tromey <tom@tromey.com>
Date:   Mon May 22 18:43:59 2017 -0600

    Simplify print_scalar_formatted
    
    This unifies the two switches in print_scalar_formatted, removing some
    now-redundant code.  Now scalar types are never converted to LONGEST,
    instead printing is done using print_*_chars, operating on the byte
    representation.
    
    ChangeLog
    2017-06-05  Tom Tromey  <tom@tromey.com>
    
            * printcmd.c (print_scalar_formatted): Unify the two switches.
            Don't convert scalars to LONGEST.
    
    2017-06-11  Tom Tromey  <tom@tromey.com>
    
            * gdb.arch/altivec-regs.exp: Expect decimal results for uint128.
Pedro Alves - June 12, 2017, 6:26 p.m.
On 06/12/2017 03:34 PM, Tom Tromey wrote:

> Here's an updated version of patch #2 from the series.
> With this the series tests cleanly on the buildbot.

Thanks.  Looks good.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 27d7162..a0dc332 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@ 
 2017-06-05  Tom Tromey  <tom@tromey.com>
 
+	* printcmd.c (print_scalar_formatted): Unify the two switches.
+	Don't convert scalars to LONGEST.
+
+2017-06-05  Tom Tromey  <tom@tromey.com>
+
 	PR exp/16225:
 	* valprint.h (print_decimal_chars): Update.
 	* valprint.c (maybe_negate_by_bytes): New function.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 84f41f5..152c2c6 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -356,47 +356,12 @@  print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
 			int size, struct ui_file *stream)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  LONGEST val_long = 0;
   unsigned int len = TYPE_LENGTH (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
   /* String printing should go through val_print_scalar_formatted.  */
   gdb_assert (options->format != 's');
 
-  if (len > sizeof(LONGEST)
-      && (TYPE_CODE (type) == TYPE_CODE_INT
-	  || TYPE_CODE (type) == TYPE_CODE_ENUM))
-    {
-      switch (options->format)
-	{
-	case 'o':
-	  print_octal_chars (stream, valaddr, len, byte_order);
-	  return;
-	case 'u':
-	case 'd':
-	  print_decimal_chars (stream, valaddr, len, !TYPE_UNSIGNED (type),
-			       byte_order);
-	  return;
-	case 't':
-	  print_binary_chars (stream, valaddr, len, byte_order, size > 0);
-	  return;
-	case 'x':
-	  print_hex_chars (stream, valaddr, len, byte_order, size > 0);
-	  return;
-	case 'z':
-	  print_hex_chars (stream, valaddr, len, byte_order, true);
-	  return;
-	case 'c':
-	  print_char_chars (stream, type, valaddr, len, byte_order);
-	  return;
-	default:
-	  break;
-	};
-    }
-
-  if (options->format != 'f')
-    val_long = unpack_long (type, valaddr);
-
   /* If the value is a pointer, and pointers and addresses are not the
      same, then at this point, the value's length (in target bytes) is
      gdbarch_addr_bit/TARGET_CHAR_BIT, not TYPE_LENGTH (type).  */
@@ -406,58 +371,93 @@  print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
   /* If we are printing it as unsigned, truncate it in case it is actually
      a negative signed value (e.g. "print/u (short)-1" should print 65535
      (if shorts are 16 bits) instead of 4294967295).  */
-  if (options->format != 'd' || TYPE_UNSIGNED (type))
+  if (options->format != 'c'
+      && (options->format != 'd' || TYPE_UNSIGNED (type)))
     {
-      if (len < sizeof (LONGEST))
-	val_long &= ((LONGEST) 1 << HOST_CHAR_BIT * len) - 1;
+      if (len < TYPE_LENGTH (type) && byte_order == BFD_ENDIAN_BIG)
+	valaddr += TYPE_LENGTH (type) - len;
     }
 
-  switch (options->format)
+  if (size != 0 && (options->format == 'x' || options->format == 't'))
     {
-    case 'x':
-      if (!size)
+      /* Truncate to fit.  */
+      unsigned newlen;
+      switch (size)
 	{
-	  /* No size specified, like in print.  Print varying # of digits.  */
-	  print_longest (stream, 'x', 1, val_long);
+	case 'b':
+	  newlen = 1;
+	  break;
+	case 'h':
+	  newlen = 2;
+	  break;
+	case 'w':
+	  newlen = 4;
+	  break;
+	case 'g':
+	  newlen = 8;
+	  break;
+	default:
+	  error (_("Undefined output size \"%c\"."), size);
 	}
-      else
-	switch (size)
-	  {
-	  case 'b':
-	  case 'h':
-	  case 'w':
-	  case 'g':
-	    print_longest (stream, size, 1, val_long);
-	    break;
-	  default:
-	    error (_("Undefined output size \"%c\"."), size);
-	  }
-      break;
+      if (newlen < len && byte_order == BFD_ENDIAN_BIG)
+	valaddr += len - newlen;
+      len = newlen;
+    }
 
-    case 'd':
-      print_longest (stream, 'd', 1, val_long);
-      break;
+  /* Historically gdb has printed floats by first casting them to a
+     long, and then printing the long.  PR cli/16242 suggests changing
+     this to using C-style hex float format.  */
+  std::vector<gdb_byte> converted_float_bytes;
+  if (TYPE_CODE (type) == TYPE_CODE_FLT
+      && (options->format == 'o'
+	  || options->format == 'x'
+	  || options->format == 't'
+	  || options->format == 'z'))
+    {
+      LONGEST val_long = unpack_long (type, valaddr);
+      converted_float_bytes.resize (TYPE_LENGTH (type));
+      store_signed_integer (converted_float_bytes.data (), TYPE_LENGTH (type),
+			    byte_order, val_long);
+      valaddr = converted_float_bytes.data ();
+    }
 
+  switch (options->format)
+    {
+    case 'o':
+      print_octal_chars (stream, valaddr, len, byte_order);
+      break;
     case 'u':
-      print_longest (stream, 'u', 0, val_long);
+      print_decimal_chars (stream, valaddr, len, false, byte_order);
       break;
-
-    case 'o':
-      print_longest (stream, 'o', 1, val_long);
+    case 0:
+    case 'd':
+      if (TYPE_CODE (type) != TYPE_CODE_FLT)
+	{
+	  print_decimal_chars (stream, valaddr, len, !TYPE_UNSIGNED (type),
+			       byte_order);
+	  break;
+	}
+      /* FALLTHROUGH */
+    case 'f':
+      type = float_type_from_length (type);
+      print_floating (valaddr, type, stream);
       break;
 
-    case 'a':
-      {
-	CORE_ADDR addr = unpack_pointer (type, valaddr);
-
-	print_address (gdbarch, addr, stream);
-      }
+    case 't':
+      print_binary_chars (stream, valaddr, len, byte_order, size > 0);
+      break;
+    case 'x':
+      print_hex_chars (stream, valaddr, len, byte_order, size > 0);
+      break;
+    case 'z':
+      print_hex_chars (stream, valaddr, len, byte_order, true);
       break;
-
     case 'c':
       {
 	struct value_print_options opts = *options;
 
+	LONGEST val_long = unpack_long (type, valaddr);
+
 	opts.format = 0;
 	if (TYPE_UNSIGNED (type))
 	  type = builtin_type (gdbarch)->builtin_true_unsigned_char;
@@ -468,66 +468,14 @@  print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       }
       break;
 
-    case 'f':
-      type = float_type_from_length (type);
-      print_floating (valaddr, type, stream);
-      break;
-
-    case 0:
-      internal_error (__FILE__, __LINE__,
-		      _("failed internal consistency check"));
-
-    case 't':
-      /* Binary; 't' stands for "two".  */
+    case 'a':
       {
-	char bits[8 * (sizeof val_long) + 1];
-	char buf[8 * (sizeof val_long) + 32];
-	char *cp = bits;
-	int width;
-
-	if (!size)
-	  width = 8 * (sizeof val_long);
-	else
-	  switch (size)
-	    {
-	    case 'b':
-	      width = 8;
-	      break;
-	    case 'h':
-	      width = 16;
-	      break;
-	    case 'w':
-	      width = 32;
-	      break;
-	    case 'g':
-	      width = 64;
-	      break;
-	    default:
-	      error (_("Undefined output size \"%c\"."), size);
-	    }
+	CORE_ADDR addr = unpack_pointer (type, valaddr);
 
-	bits[width] = '\0';
-	while (width-- > 0)
-	  {
-	    bits[width] = (val_long & 1) ? '1' : '0';
-	    val_long >>= 1;
-	  }
-	if (!size)
-	  {
-	    while (*cp && *cp == '0')
-	      cp++;
-	    if (*cp == '\0')
-	      cp--;
-	  }
-	strncpy (buf, cp, sizeof (bits));
-	fputs_filtered (buf, stream);
+	print_address (gdbarch, addr, stream);
       }
       break;
 
-    case 'z':
-      print_hex_chars (stream, valaddr, len, byte_order, true);
-      break;
-
     default:
       error (_("Undefined output format \"%c\"."), options->format);
     }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 8ddac54..854cbf0 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@ 
+2017-06-11  Tom Tromey  <tom@tromey.com>
+
+	* gdb.arch/altivec-regs.exp: Expect decimal results for uint128.
+
 2017-06-05  Tom Tromey  <tom@tromey.com>
 
 	PR exp/16225:
diff --git a/gdb/testsuite/gdb.arch/altivec-regs.exp b/gdb/testsuite/gdb.arch/altivec-regs.exp
index cc679b2..b3b5aed 100644
--- a/gdb/testsuite/gdb.arch/altivec-regs.exp
+++ b/gdb/testsuite/gdb.arch/altivec-regs.exp
@@ -113,9 +113,9 @@  gdb_test "info reg vscr" "vscr.*0x1\t1" "info reg vscr"
 # the way gdb works.
 
 if {$endianness == "big"} {
-     set decimal_vector ".uint128 = 0x1000000010000000100000001, v4_float = .1.*e-45, 1.*e-45, 1.*e-45, 1.*e-45., v4_int32 = .1, 1, 1, 1., v8_int16 = .0, 1, 0, 1, 0, 1, 0, 1., v16_int8 = .0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1.."
+     set decimal_vector ".uint128 = 79228162532711081671548469249, v4_float = .1.*e-45, 1.*e-45, 1.*e-45, 1.*e-45., v4_int32 = .1, 1, 1, 1., v8_int16 = .0, 1, 0, 1, 0, 1, 0, 1., v16_int8 = .0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1.."
 } else {
-     set decimal_vector ".uint128 = 0x1000000010000000100000001, v4_float = .1.*e-45, 1.*e-45, 1.*e-45, 1.*e-45., v4_int32 = .1, 1, 1, 1., v8_int16 = .1, 0, 1, 0, 1, 0, 1, 0., v16_int8 = .1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0.."
+     set decimal_vector ".uint128 = 79228162532711081671548469249, v4_float = .1.*e-45, 1.*e-45, 1.*e-45, 1.*e-45., v4_int32 = .1, 1, 1, 1., v8_int16 = .1, 0, 1, 0, 1, 0, 1, 0., v16_int8 = .1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0.."
 }
 
 for {set i 0} {$i < 32} {incr i 1} {