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 binop support


On Tuesday, August 10, 2010 09:06:14 pm Tom Tromey wrote:
> >>>>> "Ken" == Ken Werner <ken@linux.vnet.ibm.com> writes:
> Ken> This patch implements some binary operations for GNU vectors by
> Ken> traversing through the vector and calling scalar_binop (the former
> Ken> value_binop) element wise.
> 
> Thanks for tackling this.
> 
> This patch looks good to me.  I noticed a few nits, and one slightly
> more serious problem.

Thank you for reviewing the patch.

> Ken>  struct value *
> Ken> -value_binop (struct value *arg1, struct value *arg2, enum exp_opcode
> op) Ken> +scalar_binop (struct value *arg1, struct value *arg2, enum
> exp_opcode op)
> 
> Should be static.
> 
> Ken> +struct value *
> Ken> +vector_binop (struct value *val1, struct value *val2, enum exp_opcode
> op)
> 
> Likewise.

I agree. Those functions should be file static since value.h only exposes 
value_binop.

> Ken> +  if (TYPE_CODE (eltype1) != TYPE_CODE (eltype2))
> Ken> +    error (_("The vectors have different types"));
> Ken> +
> Ken> +  elsize =  TYPE_LENGTH (eltype1);
> 
> Extra space after the "=".

Fixed.

> Ken> +  n = TYPE_LENGTH (type1) / elsize;
> Ken> +  val = allocate_value (type1);
> Ken> +
> Ken> +  mark = value_mark ();
> Ken> +  for (i = 0; i < n; i++)
> Ken> +    {
> Ken> +      tmp = value_binop (value_subscript (val1, i),
> Ken> +			 value_subscript (val2, i), op);
> Ken> +      memcpy (value_contents_writeable (val) + i * elsize,
> Ken> +	      value_contents_all (tmp),
> Ken> +	      elsize);
> 
> It seems to me that this could use some more checking.
> 
> The TYPE_CODEs could be equal but still give the wrong answer here.  For
> example, 'int' and 'long' will have the same TYPE_CODE, but different
> sizes.  Depending on the order of the addition, this could cause the
> memcpy to overflow the destination buffer.

I've added checks for:
 - the length of the target types
 - the unsigned flag of the target types
 - and the size of the vectors as Jan suggested.

> Ken> +set testfile "gnu_vector"
> Ken> +set srcfile ${testfile}.c
> Ken> +set binfile ${objdir}/${subdir}/${testfile}
> Ken> +
> Ken> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug}] != "" } { Ken> +    untested "Couldn't compile
> ${srcfile}"
> Ken> +    return -1
> Ken> +}
> Ken> +
> Ken> +if [get_compiler_info ${binfile}] {
> Ken> +    return -1
> Ken> +}
> Ken> +
> Ken> +gdb_exit
> Ken> +gdb_start
> Ken> +gdb_reinitialize_dir $srcdir/$subdir
> Ken> +gdb_load $binfile
> 
> I think most of this (except the get_compiler_info bit) can be replaced
> with prepare_for_testing.

OK, done - thanks for the hint. In addition I've added some tests that check 
the various error conditions.

Attached is the revised patch.

Regards
Ken
ChangeLog:

2010-08-11  Ken Werner  <ken.werner@de.ibm.com>

	* gdb/valarith.c (vector_binop): New function.
	(scalar_binop): Likewise.
	(value_binop): Call scalar_binop or vector_binop depending on the types.
	* gdb/eval.c (ptrmath_type_p): Return 0 in case of TYPE_VECTOR.
	(evaluate_subexp_with_coercion): Add vector check to not convert vectors to
	pointers.
	* gdb/value.c (coerce_array): Add vector check to not coerce vectors.

testsuite/ChangeLog:

2010-08-11  Ken Werner  <ken.werner@de.ibm.com>

	* gdb.base/Makefile.in (EXECUTABLES): Add gnu_vector.
	* gdb.base/gnu_vector.c: New File.
	* gdb.base/gnu_vector.exp: Likewise.


Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.138
diff -p -u -r1.138 eval.c
--- eval.c	21 Jul 2010 14:59:05 -0000	1.138
+++ eval.c	11 Aug 2010 11:34:04 -0000
@@ -736,7 +736,7 @@ ptrmath_type_p (const struct language_de
       return 1;
 
     case TYPE_CODE_ARRAY:
-      return lang->c_style_arrays;
+      return TYPE_VECTOR (type) ? 0 : lang->c_style_arrays;
 
     default:
       return 0;
@@ -2956,6 +2956,7 @@ evaluate_subexp_with_coercion (struct ex
       var = exp->elts[pc + 2].symbol;
       type = check_typedef (SYMBOL_TYPE (var));
       if (TYPE_CODE (type) == TYPE_CODE_ARRAY
+	  && !TYPE_VECTOR (type)
 	  && CAST_IS_CONVERSION (exp->language_defn))
 	{
 	  (*pos) += 4;
Index: valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.85
diff -p -u -r1.85 valarith.c
--- valarith.c	7 Jun 2010 16:11:31 -0000	1.85
+++ valarith.c	11 Aug 2010 11:34:04 -0000
@@ -929,8 +929,8 @@ value_args_as_decimal (struct value *arg
    Does not support addition and subtraction on pointers;
    use value_ptradd, value_ptrsub or value_ptrdiff for those operations.  */
 
-struct value *
-value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
+static struct value *
+scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 {
   struct value *val;
   struct type *type1, *type2, *result_type;
@@ -1379,6 +1379,71 @@ value_binop (struct value *arg1, struct 
 
   return val;
 }
+
+/* Performs a binary operation on two vector operands by calling scalar_binop
+   for each pair of vector components.  */
+
+static struct value *
+vector_binop (struct value *val1, struct value *val2, enum exp_opcode op)
+{
+  struct value *val, *tmp, *mark;
+  struct type *type1, *type2, *eltype1, *eltype2, *result_type;
+  int t1_is_vec, t2_is_vec, elsize, n, i;
+
+  type1 = check_typedef (value_type (val1));
+  type2 = check_typedef (value_type (val2));
+
+  t1_is_vec = (TYPE_CODE (type1) == TYPE_CODE_ARRAY
+	       && TYPE_VECTOR (type1)) ? 1 : 0;
+  t2_is_vec = (TYPE_CODE (type2) == TYPE_CODE_ARRAY
+	       && TYPE_VECTOR (type2)) ? 1 : 0;
+
+  if (!t1_is_vec || !t2_is_vec)
+    error (_("Vector operations are only supported among vectors"));
+
+  eltype1 = check_typedef (TYPE_TARGET_TYPE (type1));
+  eltype2 = check_typedef (TYPE_TARGET_TYPE (type2));
+
+  if (TYPE_CODE (eltype1) != TYPE_CODE (eltype2)
+      || TYPE_LENGTH (eltype1) != TYPE_LENGTH (eltype2)
+      || TYPE_UNSIGNED (eltype1) != TYPE_UNSIGNED (eltype2))
+    error (_("Cannot perform operation on vectors with different types"));
+
+  elsize = TYPE_LENGTH (eltype1);
+  n = TYPE_LENGTH (type1) / elsize;
+
+  if (n != TYPE_LENGTH (type2) / TYPE_LENGTH (eltype2))
+    error (_("Cannot perform operation on vectors with different sizes"));
+
+  val = allocate_value (type1);
+  mark = value_mark ();
+  for (i = 0; i < n; i++)
+    {
+      tmp = value_binop (value_subscript (val1, i),
+			 value_subscript (val2, i), op);
+      memcpy (value_contents_writeable (val) + i * elsize,
+	      value_contents_all (tmp),
+	      elsize);
+     }
+  value_free_to_mark (mark);
+
+  return val;
+}
+
+/* Perform a binary operation on two operands.  */
+
+struct value *
+value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
+{
+  struct type *type1 = check_typedef (value_type (arg1));
+  struct type *type2 = check_typedef (value_type (arg2));
+
+  if ((TYPE_CODE (type1) == TYPE_CODE_ARRAY && TYPE_VECTOR (type1))
+	  || (TYPE_CODE (type2) == TYPE_CODE_ARRAY && TYPE_VECTOR (type2)))
+    return vector_binop (arg1, arg2, op);
+  else
+    return scalar_binop (arg1, arg2, op);
+}
 
 /* Simulate the C operator ! -- return 1 if ARG1 contains zero.  */
 
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.108
diff -p -u -r1.108 value.c
--- value.c	7 Jul 2010 16:15:18 -0000	1.108
+++ value.c	11 Aug 2010 11:34:04 -0000
@@ -2407,7 +2407,7 @@ coerce_array (struct value *arg)
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_ARRAY:
-      if (current_language->c_style_arrays)
+      if (!TYPE_VECTOR (type) && current_language->c_style_arrays)
 	arg = value_coerce_array (arg);
       break;
     case TYPE_CODE_FUNC:
Index: testsuite/gdb.base/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/Makefile.in,v
retrieving revision 1.6
diff -p -u -r1.6 Makefile.in
--- testsuite/gdb.base/Makefile.in	23 Apr 2010 12:08:07 -0000	1.6
+++ testsuite/gdb.base/Makefile.in	11 Aug 2010 11:34:05 -0000
@@ -13,7 +13,7 @@ EXECUTABLES = all-types annota1 bitfield
 	solib solib_sl so-impl-ld so-indr-cl \
 	step-line step-test structs structs2 \
 	twice-tmp varargs vforked-prog watchpoint whatis catch-syscall \
-	pr10179
+	pr10179 gnu_vector
 
 MISCELLANEOUS = coremmap.data ../foobar.baz \
 	shr1.sl shr2.sl solib_sl.sl solib1.sl solib2.sl
Index: testsuite/gdb.base/gnu_vector.c
===================================================================
RCS file: testsuite/gdb.base/gnu_vector.c
diff -N testsuite/gdb.base/gnu_vector.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gnu_vector.c	11 Aug 2010 11:34:05 -0000
@@ -0,0 +1,34 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Contributed by Ken Werner <ken.werner@de.ibm.com>  */
+
+int __attribute__ ((vector_size (4 * sizeof(int)))) i4a = {2, 4, 8, 16};
+int __attribute__ ((vector_size (4 * sizeof(int)))) i4b = {1, 2, 8, 4};
+float __attribute__ ((vector_size (4 * sizeof(float)))) f4a = {2, 4, 8, 16};
+float __attribute__ ((vector_size (4 * sizeof(float)))) f4b = {1, 2, 8, 4};
+unsigned int __attribute__ ((vector_size (4 * sizeof(unsigned int)))) ui4 = {2, 4, 8, 16};
+int __attribute__ ((vector_size (2 * sizeof(int)))) i2 = {1, 2};
+long long __attribute__ ((vector_size (2 * sizeof(long long)))) ll2 = {1, 2};
+float __attribute__ ((vector_size (2 * sizeof(float)))) f2 = {1, 2};
+double __attribute__ ((vector_size (2 * sizeof(double)))) d2 = {1, 2};
+
+int
+main ()
+{
+  return 0;
+}
Index: testsuite/gdb.base/gnu_vector.exp
===================================================================
RCS file: testsuite/gdb.base/gnu_vector.exp
diff -N testsuite/gdb.base/gnu_vector.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/gnu_vector.exp	11 Aug 2010 11:34:05 -0000
@@ -0,0 +1,94 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+#
+# Contributed by Ken Werner <ken.werner@de.ibm.com>.
+#
+# Tests GDBs support for GNU vectors.
+# http://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+set testfile "gnu_vector"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+# Check if our compiler is a GCC that suppports the vector extension
+if { ![test_compiler_info gcc-4-*] } {
+    setup_xfail "*-*-*"
+    fail "This compiler can not handle GNU vectors"
+    return 0
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug}] } {
+    return -1
+}
+
+if { ![runto main] } {
+    fail "runto main"
+    return -1
+}
+
+# Test binary 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
+gdb_test "print i4a + i4b" "\\\$$decimal = \\{3, 6, 16, 20\\}"
+gdb_test "print i4a - i4b" "\\\$$decimal = \\{1, 2, 0, 12\\}"
+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\\}"
+# 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\\}"
+# 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
+gdb_test "print f4a" "\\\$$decimal = \\{2, 4, 8, 16\\}"
+gdb_test "print f4b" "\\\$$decimal = \\{1, 2, 8, 4\\}"
+# Arithmetic operators
+gdb_test "print f4a + f4b" "\\\$$decimal = \\{3, 6, 16, 20\\}"
+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\\}"
+
+# Test error conditions
+gdb_test "print i4a + 1" "Vector operations are only supported among vectors"
+gdb_test "print 1 + f4a" "Vector operations are only supported among vectors"
+gdb_test "print i4a + d2" "Cannot perform operation on vectors with different types"
+gdb_test "print d2 + i4a" "Cannot perform operation on vectors with different types"
+gdb_test "print f4a + ll2" "Cannot perform operation on vectors with different types"
+gdb_test "print ll2 + f4a" "Cannot perform operation on vectors with different types"
+gdb_test "print i2 + ll2" "Cannot perform operation on vectors with different types"
+gdb_test "print ll2 + i2" "Cannot perform operation on vectors with different types"
+gdb_test "print i4a + ll2" "Cannot perform operation on vectors with different types"
+gdb_test "print ll2 + i4a" "Cannot perform operation on vectors with different types"
+gdb_test "print f4a + d2" "Cannot perform operation on vectors with different types"
+gdb_test "print d2 + f4a" "Cannot perform operation on vectors with different types"
+gdb_test "print ui4 + i4a" "Cannot perform operation on vectors with different types"
+gdb_test "print i4a + ui4" "Cannot perform operation on vectors with different types"
+gdb_test "print i4a + i2" "Cannot perform operation on vectors with different sizes"
+gdb_test "print i2 + i4a" "Cannot perform operation on vectors with different sizes"
+gdb_test "print f4a + f2" "Cannot perform operation on vectors with different sizes"
+gdb_test "print f2 + f4a" "Cannot perform operation on vectors with different sizes"
+

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