[rs6000] Fix invalid type returned from builtin vec_extract
diff mbox series

Message ID 6e80e413-74ed-ff11-9474-fa11a6af1b92@linux.ibm.com
State New
Headers show
Series
  • [rs6000] Fix invalid type returned from builtin vec_extract
Related show

Commit Message

Kelvin Nilsen Jan. 28, 2019, 6:11 p.m. UTC
An error in the type returned from the built-in vec_extract function was recently reported, as represented in the following sample program:

#include <stdio.h>
#include <altivec.h>

int main() {
    unsigned char uc = 0xf6;
    printf("explicit cast: %x\n", (int)uc);

    vector unsigned char v = vec_splats((unsigned char)0xf6);
    printf("cast from vec_extract(): %x\n", (int)vec_extract(v, 0));
    return 0;
}

When compiled with the current trunk, the output of running this program is:

$ ./a.out 
explicit cast: f6
cast from vec_extract(): fffffff6

The fix is to coerce the result of vec_extract so that it matches the type of the array element supplied as its first argument.

I have built and regression tested this patch on powerpc64le-unknown-linux with no regressions.  Is this ok for trunk?

gcc/ChangeLog:

2019-01-28  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* config/rs6000/rs6000-c.c (altivec-resolve_overloaded_builtin):
	Change handling of ALTIVEC_BUILTIN_VEC_EXTRACT.  Coerce result to
	type of vector element when vec_extract is implemented by direct
	move.

gcc/testsuite/ChangeLog:

2019-01-28  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* gcc.target/powerpc/vec-extract-schar-1.c: New test.
	* gcc.target/powerpc/vec-extract-sint-1.c: New test.
	* gcc.target/powerpc/vec-extract-sint128-1.c: New test.
	* gcc.target/powerpc/vec-extract-slong-1.c: New test.
	* gcc.target/powerpc/vec-extract-sshort-1.c: New test.
	* gcc.target/powerpc/vec-extract-uchar-1.c: New test.
	* gcc.target/powerpc/vec-extract-uint-1.c: New test.
	* gcc.target/powerpc/vec-extract-uint128-1.c: New test.
	* gcc.target/powerpc/vec-extract-ulong-1.c: New test.
	* gcc.target/powerpc/vec-extract-ushort-1.c: New test.

Comments

Segher Boessenkool Jan. 28, 2019, 6:43 p.m. UTC | #1
Hi Kelvin,

On Mon, Jan 28, 2019 at 12:11:34PM -0600, Kelvin Nilsen wrote:
> An error in the type returned from the built-in vec_extract function was recently reported, as represented in the following sample program:

[ snip ]

> The fix is to coerce the result of vec_extract so that it matches the type of the array element supplied as its first argument.

The patch is fine, but there are some issues with the new testcases:

> --- gcc/testsuite/gcc.target/powerpc/vec-extract-schar-1.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/vec-extract-schar-1.c	(working copy)
> @@ -0,0 +1,27 @@
> +/* Test to verify that the vec_extract from a vector of
> +   signed chars remains signed.  */
> +/* { dg-do run } */
> +/* { dg-options "-ansi -mcpu=power8 " } */

(stray space)

You need to also have

/* { dg-require-effective-target powerpc_p8vector_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */

(the first so that the builtins exist, the second so that things work if
you set -mcpu= in RUNTESTFLAGS, as is common).

Could you test with that please?

Okay for trunk with that.  Also okay for backport where wanted/needed.

Thanks!


Segher

Patch
diff mbox series

Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 268196)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -6645,7 +6645,13 @@ 
 	    }
 
 	  if (call)
-	    return build_call_expr (call, 2, arg1, arg2);
+	    {
+	      tree result = build_call_expr (call, 2, arg1, arg2);
+	      /* Coerce the result to vector element type.  May be no-op.  */
+	      arg1_inner_type = TREE_TYPE (arg1_type);
+	      result = fold_convert (arg1_inner_type, result);
+	      return result;
+	    }
 	}
 
       /* Build *(((arg1_inner_type*)&(vector type){arg1})+arg2). */
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-schar-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-schar-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-schar-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed chars remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(signed char sc) {
+  int sce;
+
+  vector signed char v = vec_splats(sc);
+  sce = vec_extract(v,0);
+
+  if (sce != sc)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf6);
+  test1 (0x76);
+  test1 (0x06);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-sint-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-sint-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-sint-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed ints remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(signed int si) {
+  long long int sie;
+
+  vector signed int v = vec_splats(si);
+  sie = vec_extract(v,0);
+
+  if (sie != si)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf6000000);
+  test1 (0x76000000);
+  test1 (0x06000000);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-sint128-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-sint128-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-sint128-1.c	(working copy)
@@ -0,0 +1,25 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed __int128s remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(signed __int128 st) {
+
+  vector signed long long int v = vec_splats(st);
+
+  if (vec_extract (v, 0) > st)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (((__int128) 0xf600000000000000LL) << 64);
+  test1 (((__int128) 0x7600000000000000LL) << 64);
+  test1 (((__int128) 0x0600000000000000LL) << 64);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-slong-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-slong-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-slong-1.c	(working copy)
@@ -0,0 +1,25 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed longs remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(signed long long int sl) {
+
+  vector signed long long int v = vec_splats(sl);
+
+  if (vec_extract (v, 0) > sl)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf600000000000000LL);
+  test1 (0x7600000000000000LL);
+  test1 (0x0600000000000000LL);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-sshort-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-sshort-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-sshort-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed shorts remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(signed short ss) {
+  int sse;
+
+  vector signed short v = vec_splats(ss);
+  sse = vec_extract(v,0);
+
+  if (sse != ss)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf600);
+  test1 (0x7600);
+  test1 (0x0600);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-uchar-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-uchar-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-uchar-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   unsigned chars remains unsigned.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(unsigned char uc) {
+  int uce;
+
+  vector unsigned char v = vec_splats(uc);
+  uce = vec_extract(v,0);
+
+  if (uce != uc)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf6);
+  test1 (0x76);
+  test1 (0x06);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-uint-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-uint-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-uint-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   unsigned ints remains unsigned.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(unsigned int ui) {
+  long long int uie;
+
+  vector unsigned int v = vec_splats(ui);
+  uie = vec_extract(v,0);
+
+  if (uie != ui)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf6000000);
+  test1 (0x76000000);
+  test1 (0x06000000);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-uint128-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-uint128-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-uint128-1.c	(working copy)
@@ -0,0 +1,25 @@ 
+/* Test to verify that the vec_extract from a vector of
+   unsigned __int128s remains unsigned.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(unsigned __int128 ul) {
+
+  vector unsigned __int128 v = vec_splats(ul);
+
+  if (vec_extract (v, 0) < ul)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (((__int128) 0xf600000000000000LL) << 64);
+  test1 (((__int128) 0x7600000000000000LL) << 64);
+  test1 (((__int128) 0x0600000000000000LL) << 64);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-ulong-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-ulong-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-ulong-1.c	(working copy)
@@ -0,0 +1,25 @@ 
+/* Test to verify that the vec_extract from a vector of
+   unsigned longs remains unsigned.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(unsigned long long int ul) {
+
+  vector unsigned long long int v = vec_splats(ul);
+
+  if (vec_extract (v, 0) < ul)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf600000000000000LL);
+  test1 (0x7600000000000000LL);
+  test1 (0x0600000000000000LL);
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-extract-ushort-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-extract-ushort-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-ushort-1.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* Test to verify that the vec_extract from a vector of
+   signed shorts remains signed.  */
+/* { dg-do run } */
+/* { dg-options "-ansi -mcpu=power8 " } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int test1(unsigned short us) {
+  int use;
+
+  vector unsigned short v = vec_splats(us);
+  use = vec_extract(v,0);
+
+  if (use != us)
+    abort();
+  return 0;
+}
+
+int main()
+{
+  test1 (0xf600);
+  test1 (0x7600);
+  test1 (0x0600);
+  return 0;
+}