[9/9] Remove unnecessary copies of variables in read/write_pieced_value

Message ID 1491586736-21296-10-git-send-email-arnez@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez April 7, 2017, 5:38 p.m. UTC
  In read_pieced_value's main loop, the variables `dest_offset_bits' and
`source_offset_bits' are basically just copies of `offset' and
`bits_to_skip', respectively.  In write_pieced_value the copies are
reversed.  This is not very helpful when trying to keep the logic between
these functions in sync.  Since the copies are unnecessary, this patch
just removes them.

gdb/ChangeLog:

	* dwarf2loc.c (read_pieced_value): Remove unnecessary variables
	dest_offset_bits and source_offset_bits.
	(write_pieced_value): Likewise.
---
 gdb/dwarf2loc.c | 118 +++++++++++++++++++++++++-------------------------------
 1 file changed, 52 insertions(+), 66 deletions(-)
  

Comments

Simon Marchi April 14, 2017, 3:21 p.m. UTC | #1
On 2017-04-07 13:38, Andreas Arnez wrote:
> In read_pieced_value's main loop, the variables `dest_offset_bits' and
> `source_offset_bits' are basically just copies of `offset' and
> `bits_to_skip', respectively.  In write_pieced_value the copies are
> reversed.  This is not very helpful when trying to keep the logic 
> between
> these functions in sync.  Since the copies are unnecessary, this patch
> just removes them.

I agree about dest_offset_bits being the same as offset in 
read_pieced_value and source_offset_bits being the same as offset in 
write_pieced_value, I had also identified this as a potential 
simplification.  But I'd keep the names 
source_offset_bits/dest_offset_bits rather than offset.  Or maybe 
offset_in_value_bits, something like that.

Also, it might be clearer if bits_to_skip was named source_bits_to_skip 
in read_pieced_value and dest_bits_to_skip in write_pieced_value, to 
clarify what is skipped exactly.

Thanks,

Simon
  

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 045c2d3..d7bd166 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1790,25 +1790,16 @@  read_pieced_value (struct value *v)
   else
     type_len = 8 * TYPE_LENGTH (value_type (v));
 
-  for (i = 0; i < c->n_pieces && offset < type_len; i++)
+  /* Advance to the first non-skipped piece.  */
+  for (i = 0; i < c->n_pieces && bits_to_skip >= c->pieces[i].size; i++)
+    bits_to_skip -= c->pieces[i].size;
+
+  for (; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
       size_t this_size, this_size_bits;
-      long dest_offset_bits, source_offset_bits;
-
-      /* Compute size, source, and destination offsets for copying, in
-	 bits.  */
-      this_size_bits = p->size;
-      if (bits_to_skip > 0 && bits_to_skip >= this_size_bits)
-	{
-	  bits_to_skip -= this_size_bits;
-	  continue;
-	}
-      source_offset_bits = bits_to_skip;
-      this_size_bits -= bits_to_skip;
-      bits_to_skip = 0;
-      dest_offset_bits = offset;
 
+      this_size_bits = p->size - bits_to_skip;
       if (this_size_bits > type_len - offset)
 	this_size_bits = type_len - offset;
 
@@ -1827,15 +1818,15 @@  read_pieced_value (struct value *v)
 		&& p->offset + p->size < reg_bits)
 	      {
 		/* Big-endian, and we want less than full size.  */
-		source_offset_bits += reg_bits - (p->offset + p->size);
+		bits_to_skip += reg_bits - (p->offset + p->size);
 	      }
 	    else
-	      source_offset_bits += p->offset;
-	    this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+	      bits_to_skip += p->offset;
+	    this_size = bits_to_bytes (bits_to_skip, this_size_bits);
 	    buffer.reserve (this_size);
 
 	    if (!get_frame_register_bytes (frame, gdb_regnum,
-					  source_offset_bits / 8,
+					  bits_to_skip / 8,
 					  this_size, buffer.data (),
 					  &optim, &unavail))
 	      {
@@ -1845,23 +1836,23 @@  read_pieced_value (struct value *v)
 		  mark_value_bits_unavailable (v, offset, this_size_bits);
 		break;
 	      }
-	    copy_bitwise (contents, dest_offset_bits,
-			  buffer.data (), source_offset_bits % 8,
+	    copy_bitwise (contents, offset,
+			  buffer.data (), bits_to_skip % 8,
 			  this_size_bits, bits_big_endian);
 	  }
 	  break;
 
 	case DWARF_VALUE_MEMORY:
-	  source_offset_bits += p->offset;
-	  this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+	  bits_to_skip += p->offset;
+	  this_size = bits_to_bytes (bits_to_skip, this_size_bits);
 	  buffer.reserve (this_size);
 
 	  read_value_memory (v, offset,
 			     p->v.mem.in_stack_memory,
-			     p->v.mem.addr + source_offset_bits / 8,
+			     p->v.mem.addr + bits_to_skip / 8,
 			     buffer.data (), this_size);
-	  copy_bitwise (contents, dest_offset_bits,
-			buffer.data (), source_offset_bits % 8,
+	  copy_bitwise (contents, offset,
+			buffer.data (), bits_to_skip % 8,
 			this_size_bits, bits_big_endian);
 	  break;
 
@@ -1877,12 +1868,12 @@  read_pieced_value (struct value *v)
 
 	    /* Piece is anchored at least significant bit end.  */
 	    if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
-	      source_offset_bits += obj_size - (p->offset + p->size);
+	      bits_to_skip += obj_size - (p->offset + p->size);
 	    else
-	      source_offset_bits += p->offset;
-	    copy_bitwise (contents, dest_offset_bits,
+	      bits_to_skip += p->offset;
+	    copy_bitwise (contents, offset,
 			  value_contents_all (p->v.value),
-			  source_offset_bits,
+			  bits_to_skip,
 			  this_size_bits, bits_big_endian);
 	  }
 	  break;
@@ -1893,14 +1884,14 @@  read_pieced_value (struct value *v)
 	    size_t n = this_size_bits;
 
 	    /* Cut off at the end of the implicit value.  */
-	    source_offset_bits += p->offset;
-	    if (source_offset_bits >= obj_size)
+	    bits_to_skip += p->offset;
+	    if (bits_to_skip >= obj_size)
 	      break;
-	    if (n > obj_size - source_offset_bits)
-	      n = obj_size - source_offset_bits;
+	    if (n > obj_size - bits_to_skip)
+	      n = obj_size - bits_to_skip;
 
-	    copy_bitwise (contents, dest_offset_bits,
-			  p->v.literal.data, source_offset_bits,
+	    copy_bitwise (contents, offset,
+			  p->v.literal.data, bits_to_skip,
 			  n, bits_big_endian);
 	  }
 	  break;
@@ -1919,6 +1910,7 @@  read_pieced_value (struct value *v)
 	}
 
       offset += this_size_bits;
+      bits_to_skip = 0;
     }
 }
 
@@ -1954,23 +1946,16 @@  write_pieced_value (struct value *to, struct value *from)
   else
     type_len = 8 * TYPE_LENGTH (value_type (to));
 
-  for (i = 0; i < c->n_pieces && offset < type_len; i++)
+  /* Advance to the first non-skipped piece.  */
+  for (i = 0; i < c->n_pieces && bits_to_skip >= c->pieces[i].size; i++)
+    bits_to_skip -= c->pieces[i].size;
+
+  for (; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
       size_t this_size_bits, this_size;
-      long dest_offset_bits, source_offset_bits;
-
-      this_size_bits = p->size;
-      if (bits_to_skip > 0 && bits_to_skip >= this_size_bits)
-	{
-	  bits_to_skip -= this_size_bits;
-	  continue;
-	}
-      dest_offset_bits = bits_to_skip;
-      this_size_bits -= bits_to_skip;
-      bits_to_skip = 0;
-      source_offset_bits = offset;
 
+      this_size_bits = p->size - bits_to_skip;
       if (this_size_bits > type_len - offset)
 	this_size_bits = type_len - offset;
 
@@ -1987,20 +1972,20 @@  write_pieced_value (struct value *to, struct value *from)
 		&& p->offset + p->size < reg_bits)
 	      {
 		/* Big-endian, and we want less than full size.  */
-		dest_offset_bits += reg_bits - (p->offset + p->size);
+		bits_to_skip += reg_bits - (p->offset + p->size);
 	      }
 	    else
-	      dest_offset_bits += p->offset;
-	    this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+	      bits_to_skip += p->offset;
+	    this_size = bits_to_bytes (bits_to_skip, this_size_bits);
 	    buffer.reserve (this_size);
 
-	    if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
+	    if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
 	      {
 		/* Need some bits from original register value.  */
 		int optim, unavail;
 
 		if (!get_frame_register_bytes (frame, gdb_regnum,
-					       dest_offset_bits / 8,
+					       bits_to_skip / 8,
 					       this_size, buffer.data (),
 					       &optim, &unavail))
 		  {
@@ -2017,34 +2002,34 @@  write_pieced_value (struct value *to, struct value *from)
 		  }
 	      }
 
-	    copy_bitwise (buffer.data (), dest_offset_bits % 8,
-			  contents, source_offset_bits,
+	    copy_bitwise (buffer.data (), bits_to_skip % 8,
+			  contents, offset,
 			  this_size_bits, bits_big_endian);
 	    put_frame_register_bytes (frame, gdb_regnum,
-				      dest_offset_bits / 8,
+				      bits_to_skip / 8,
 				      this_size, buffer.data ());
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
 	  {
-	    dest_offset_bits += p->offset;
+	    bits_to_skip += p->offset;
 
-	    CORE_ADDR start_addr = p->v.mem.addr + dest_offset_bits / 8;
+	    CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
 
-	    if (dest_offset_bits % 8 == 0 && this_size_bits % 8 == 0
-		&& source_offset_bits % 8 == 0)
+	    if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
+		&& offset % 8 == 0)
 	      {
 		/* Everything is byte-aligned; no buffer needed.  */
 		write_memory (start_addr,
-			      contents + source_offset_bits / 8,
+			      contents + offset / 8,
 			      this_size_bits / 8);
 		break;
 	      }
 
-	    this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+	    this_size = bits_to_bytes (bits_to_skip, this_size_bits);
 	    buffer.reserve (this_size);
 
-	    if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
+	    if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
 	      {
 		if (this_size <= 8)
 		  {
@@ -2061,8 +2046,8 @@  write_pieced_value (struct value *to, struct value *from)
 		  }
 	      }
 
-	    copy_bitwise (buffer.data (), dest_offset_bits % 8,
-			  contents, source_offset_bits,
+	    copy_bitwise (buffer.data (), bits_to_skip % 8,
+			  contents, offset,
 			  this_size_bits, bits_big_endian);
 	    write_memory (start_addr, buffer.data (), this_size);
 	  }
@@ -2072,6 +2057,7 @@  write_pieced_value (struct value *to, struct value *from)
 	  break;
 	}
       offset += this_size_bits;
+      bits_to_skip = 0;
     }
 }