This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] GNU vector unop support


On Thursday, September 30, 2010 8:56:34 pm Joel Brobecker wrote:
> > 	* eval.c (evaluate_subexp_standard) <UNOP_POSTINCREMENT,
> > 	UNOP_POSTDECREMENT>: Copy arg1 to prevent returning a lvalue.
> 
> Would you mind if we treated this as a separate patch, on top of
> the current patch? I'd also like to add a test that verifies this,
> hopefully not involving vector types.

Ok, good point - here we go:
http://sourceware.org/ml/gdb-patches/2010-10/msg00012.html

> You also seem to do cleanups at the same time, which is OK. But
> I'd rather they be submitted separately. It's not all that bad,
> but it does tend to pollute a bit the meat of the change you are
> trying to make.  Fortunately, tools such as git make it really easy
> to manage (I know Dan an Pedro use something different, but I just
> can't remember the name of that tool).

Thanks for the advice. The new patch changes only what I think is absolutely 
necessary in order to implement the wanted functionality.

> > +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type))
> > 
> >      {
> > 
> > -      return value_from_longest (type, value_as_long (arg1));
> > +      val = allocate_value (type);
> > +      memcpy (value_contents_raw (val), value_contents (arg1),
> > +	      TYPE_LENGTH (type));
> > 
> >      }
> 
> I'd rather you put the check for vector types at the end of the
> if/elsif sequence. I think that vector types are going to be less
> common than integers, floats et al.  This is true for all the changes
> you made.

Sounds reasonable. Fixed.

> One question: Is it possible to have a non-array vector type?
> In other words, can we just check for TYPE_VECTOR (type) instead
> of TYPE_CODE (type) and TYPE_VECTOR (type)?

No, not that I'm aware of. Even a GNU Vector with a single element only is an 
array underneath. My understanding is that querying the flag_vector is only 
legal if the type is an array. Historically vectors have been treated as 
arrays and there were only a few cases within GDB where arrays and vector were 
handled differently. There was no need to introduce a new type code. The more I 
do with vectors within GDB the more I think it would be nice to have such a 
distinct type code.

> Please move the assignment to `val' one line below (empty line after
> the declarations, and no need for an empty line between the two
> statements).

Ok.

> Unnecessary empty line?

Yep. Thanks.

> > @@ -421,7 +421,8 @@ value_cast (struct type *type, struct va
> > 
> >      }
> >    
> >    if (current_language->c_style_arrays
> > 
> > -      && TYPE_CODE (type2) == TYPE_CODE_ARRAY)
> > +      && TYPE_CODE (type2) == TYPE_CODE_ARRAY
> > +      && ! TYPE_VECTOR (type2))
> > 
> >      arg2 = value_coerce_array (arg2);
> 
> You forgot to mention this change in you ChangeLog, and I don't
> understand why it's necessary? Can you elaborate? (and add a test
> if appropriate)

Good catch. I thought that value_cast should not call value_coerce_array for 
vectors. You probably don't want a value which is a pointer to the first 
element in that case because vectors aren't arrays. Since this has nothing 
particularly to do with what the patch intended to improve I removed this 
chunk.

> Empty line after the declarations.
Fixed.

This new patch depends on the lval-fix posted here:
http://sourceware.org/ml/gdb-patches/2010-10/msg00029.html
Thanks for looking into it.

Regards
Ken Werner
ChangeLog:

2010-10-04  Ken Werner  <ken.werner@de.ibm.com>

	* valarith.c (value_pos, value_neg, value_complement): Handle
	vector types.
	* valops.c (value_one): Likewise.

testsuite/ChangeLog:

2010-10-04  Ken Werner  <ken.werner@de.ibm.com>

	* gdb.base/gnu_vector.exp: Add unary operator tests.


Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.86
diff -p -u -r1.86 valarith.c
--- gdb/valarith.c	11 Aug 2010 16:48:26 -0000	1.86
+++ gdb/valarith.c	4 Oct 2010 20:48:52 -0000
@@ -1686,6 +1686,14 @@ value_pos (struct value *arg1)
     {
       return value_from_longest (type, value_as_long (arg1));
     }
+  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type))
+    {
+      struct value *val = allocate_value (type);
+
+      memcpy (value_contents_raw (val), value_contents (arg1),
+              TYPE_LENGTH (type));
+      return val;
+    }
   else
     {
       error ("Argument to positive operation not a number.");
@@ -1723,6 +1731,20 @@ value_neg (struct value *arg1)
     {
       return value_from_longest (type, -value_as_long (arg1));
     }
+  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type))
+    {
+      struct value *tmp, *val = allocate_value (type);
+      struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type));
+      int i, n = TYPE_LENGTH (type) / TYPE_LENGTH (eltype);
+
+      for (i = 0; i < n; i++)
+	{
+	  tmp = value_neg (value_subscript (arg1, i));
+	  memcpy (value_contents_writeable (val) + i * TYPE_LENGTH (eltype),
+		  value_contents_all (tmp), TYPE_LENGTH (eltype));
+	}
+      return val;
+    }
   else
     {
       error (_("Argument to negate operation not a number."));
@@ -1734,14 +1756,31 @@ struct value *
 value_complement (struct value *arg1)
 {
   struct type *type;
+  struct value *val;
 
   arg1 = coerce_ref (arg1);
   type = check_typedef (value_type (arg1));
 
-  if (!is_integral_type (type))
-    error (_("Argument to complement operation not an integer or boolean."));
+  if (is_integral_type (type))
+    val = value_from_longest (type, ~value_as_long (arg1));
+  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type))
+    {
+      struct value *tmp;
+      struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type));
+      int i, n = TYPE_LENGTH (type) / TYPE_LENGTH (eltype);
+
+      val = allocate_value (type);
+      for (i = 0; i < n; i++)
+        {
+          tmp = value_complement (value_subscript (arg1, i));
+          memcpy (value_contents_writeable (val) + i * TYPE_LENGTH (eltype),
+                  value_contents_all (tmp), TYPE_LENGTH (eltype));
+        }
+    }
+  else
+    error (_("Argument to complement operation not an integer, boolean."));
 
-  return value_from_longest (type, ~value_as_long (arg1));
+  return val;
 }
 
 /* The INDEX'th bit of SET value whose value_type is TYPE,
Index: gdb/valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.251
diff -p -u -r1.251 valops.c
--- gdb/valops.c	24 Sep 2010 14:47:53 -0000	1.251
+++ gdb/valops.c	4 Oct 2010 20:48:52 -0000
@@ -849,6 +849,20 @@ value_one (struct type *type, enum lval_
     {
       val = value_from_longest (type, (LONGEST) 1);
     }
+  else if (TYPE_CODE (type1) == TYPE_CODE_ARRAY && TYPE_VECTOR (type1))
+    {
+      struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type1));
+      int i, n = TYPE_LENGTH (type1) / TYPE_LENGTH (eltype);
+      struct value *tmp;
+
+      val = allocate_value (type);
+      for (i = 0; i < n; i++)
+	{
+	  tmp = value_one (eltype, lv);
+	  memcpy (value_contents_writeable (val) + i * TYPE_LENGTH (eltype),
+		  value_contents_all (tmp), TYPE_LENGTH (eltype));
+	}
+    }
   else
     {
       error (_("Not a numeric type."));
Index: gdb/testsuite/gdb.base/gnu_vector.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/gnu_vector.exp,v
retrieving revision 1.1
diff -p -u -r1.1 gnu_vector.exp
--- gdb/testsuite/gdb.base/gnu_vector.exp	11 Aug 2010 16:48:26 -0000	1.1
+++ gdb/testsuite/gdb.base/gnu_vector.exp	4 Oct 2010 20:48:53 -0000
@@ -46,7 +46,7 @@ if { ![runto main] } {
     return -1
 }
 
-# Test binary operators on integer vector types
+# Test operators on integer vector types
 gdb_test "print i4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
 gdb_test "print i4b" "\\\$$decimal = \\{1, 2, 8, 4\\}"
 # Arithmetic operators
@@ -55,15 +55,23 @@ gdb_test "print i4a - i4b" "\\\$$decimal
 gdb_test "print i4a * i4b" "\\\$$decimal = \\{2, 8, 64, 64\\}"
 gdb_test "print i4a / i4b" "\\\$$decimal = \\{2, 2, 1, 4\\}"
 gdb_test "print i4a % i4b" "\\\$$decimal = \\{0, 0, 0, 0\\}"
+gdb_test "print i4a++" "\\\$$decimal = \\{2, 4, 8, 16\\}"
+gdb_test "print ++i4a" "\\\$$decimal = \\{4, 6, 10, 18\\}"
+gdb_test "print i4a--" "\\\$$decimal = \\{4, 6, 10, 18\\}"
+gdb_test "print --i4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
+gdb_test "print +i4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
+gdb_test "print -i4a" "\\\$$decimal = \\{-2, -4, -8, -16\\}"
+
 # Bitwise operators
 gdb_test "print i4a & i4b" "\\\$$decimal = \\{0, 0, 8, 0\\}"
 gdb_test "print i4a | i4b" "\\\$$decimal = \\{3, 6, 8, 20\\}"
 gdb_test "print i4a ^ i4b" "\\\$$decimal = \\{3, 6, 0, 20\\}"
+gdb_test "print ~i4a" "\\\$$decimal = \\{-3, -5, -9, -17\\}"
 # Shift operators
 gdb_test "print i4a << i4b" "\\\$$decimal = \\{4, 16, 2048, 256\\}"
 gdb_test "print i4a >> i4b" "\\\$$decimal = \\{1, 1, 0, 1\\}"
 
-# Test binary operators on floating point vector types
+# Test operators on floating point vector types
 gdb_test "print f4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
 gdb_test "print f4b" "\\\$$decimal = \\{1, 2, 8, 4\\}"
 # Arithmetic operators
@@ -71,6 +79,8 @@ gdb_test "print f4a + f4b" "\\\$$decimal
 gdb_test "print f4a - f4b" "\\\$$decimal = \\{1, 2, 0, 12\\}"
 gdb_test "print f4a * f4b" "\\\$$decimal = \\{2, 8, 64, 64\\}"
 gdb_test "print f4a / f4b" "\\\$$decimal = \\{2, 2, 1, 4\\}"
+gdb_test "print +f4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
+gdb_test "print -f4a" "\\\$$decimal = \\{-2, -4, -8, -16\\}"
 
 # Test error conditions
 gdb_test "print i4a + 1" "Vector operations are only supported among vectors"

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]