This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix powerpc ICE with __builtin_vec_ld on an array (PR target/82112, take 2)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: David Edelsohn <dje dot gcc at gmail dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 12 Sep 2017 16:25:48 +0200
- Subject: [PATCH] Fix powerpc ICE with __builtin_vec_ld on an array (PR target/82112, take 2)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jakub at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0FB65C04B938
- References: <20170907084030.GS2323@tucnak>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
On Thu, Sep 07, 2017 at 10:40:30AM +0200, Jakub Jelinek wrote:
> The C and C++ FE handle resolve_overloaded_builtin differently, the C FE
> calls it when e.g. array-to-pointer and function-to-pointer conversions
> are already done on the arguments, while C++ FE does that only much later.
> The c-common code e.g. for __sync/__atomic builtins deals with
> that e.g. by:
> if (TREE_CODE (type) == ARRAY_TYPE)
> {
> /* Force array-to-pointer decay for C++. */
> gcc_assert (c_dialect_cxx());
> (*params)[0] = default_conversion ((*params)[0]);
> type = TREE_TYPE ((*params)[0]);
> }
> while the rs6000 md hook uses default_conversion only in one spot (the
> generic handling), but for vec_ld and vec_st does something on its own.
> What is even worse is that for vec_ld, it does that too late, there is
> a fold_convert in between for the case where the element type is
> qualified, and that only works if the argument is pointer, not array
> (in which case it ICEs).
> So, the following patch moves the vec_ld conversion earlier and for both
> vec_ld and vec_st uses what c-common as well as later
> altivec_resolve_overloaded_builtin uses.
>
> Bootstrapped/regtested on powerpc64-linux (regtest with {,-m32}), ok for
> trunk?
Here is an updated version of that patch, fixed for the C90 non-lvalue
arrays (which we want to reject as c-family rejects them for __atomic_*
etc.). Bootstrapped/regtested on powerpc64-linux (regtest with {,-m32}), ok for
trunk?
2017-09-12 Jakub Jelinek <jakub@redhat.com>
PR target/82112
* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): For
ALTIVEC_BUILTIN_VEC_LD if arg1 has array type call default_conversion
on it early, rather than manual conversion late. For
ALTIVEC_BUILTIN_VEC_ST if arg2 has array type call default_conversion
instead of performing manual conversion.
* gcc.target/powerpc/pr82112.c: New test.
* g++.dg/ext/altivec-18.C: New test.
--- gcc/config/rs6000/rs6000-c.c.jj 2017-09-08 09:13:59.863591547 +0200
+++ gcc/config/rs6000/rs6000-c.c 2017-09-08 09:16:36.354738183 +0200
@@ -6480,7 +6480,13 @@ altivec_resolve_overloaded_builtin (loca
/* Strip qualifiers like "const" from the pointer arg. */
tree arg1_type = TREE_TYPE (arg1);
- if (!POINTER_TYPE_P (arg1_type) && TREE_CODE (arg1_type) != ARRAY_TYPE)
+ if (TREE_CODE (arg1_type) == ARRAY_TYPE && c_dialect_cxx ())
+ {
+ /* Force array-to-pointer decay for C++. */
+ arg1 = default_conversion (arg1);
+ arg1_type = TREE_TYPE (arg1);
+ }
+ if (!POINTER_TYPE_P (arg1_type))
goto bad;
tree inner_type = TREE_TYPE (arg1_type);
@@ -6500,15 +6506,6 @@ altivec_resolve_overloaded_builtin (loca
if (!ptrofftype_p (TREE_TYPE (arg0)))
arg0 = build1 (NOP_EXPR, sizetype, arg0);
- tree arg1_type = TREE_TYPE (arg1);
- if (TREE_CODE (arg1_type) == ARRAY_TYPE)
- {
- arg1_type = TYPE_POINTER_TO (TREE_TYPE (arg1_type));
- tree const0 = build_int_cstu (sizetype, 0);
- tree arg1_elt0 = build_array_ref (loc, arg1, const0);
- arg1 = build1 (ADDR_EXPR, arg1_type, arg1_elt0);
- }
-
tree addr = fold_build2_loc (loc, POINTER_PLUS_EXPR, arg1_type,
arg1, arg0);
tree aligned = fold_build2_loc (loc, BIT_AND_EXPR, arg1_type, addr,
@@ -6563,12 +6560,11 @@ altivec_resolve_overloaded_builtin (loca
arg1 = build1 (NOP_EXPR, sizetype, arg1);
tree arg2_type = TREE_TYPE (arg2);
- if (TREE_CODE (arg2_type) == ARRAY_TYPE)
+ if (TREE_CODE (arg2_type) == ARRAY_TYPE && c_dialect_cxx ())
{
- arg2_type = TYPE_POINTER_TO (TREE_TYPE (arg2_type));
- tree const0 = build_int_cstu (sizetype, 0);
- tree arg2_elt0 = build_array_ref (loc, arg2, const0);
- arg2 = build1 (ADDR_EXPR, arg2_type, arg2_elt0);
+ /* Force array-to-pointer decay for C++. */
+ arg2 = default_conversion (arg2);
+ arg2_type = TREE_TYPE (arg2);
}
/* Find the built-in to make sure a compatible one exists; if not
--- gcc/testsuite/gcc.target/powerpc/pr82112.c.jj 2017-09-08 09:20:35.187912843 +0200
+++ gcc/testsuite/gcc.target/powerpc/pr82112.c 2017-09-08 09:24:21.362376676 +0200
@@ -0,0 +1,16 @@
+/* PR target/82112 */
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -std=gnu90" } */
+
+#include <altivec.h>
+
+struct __attribute__((aligned (16))) S { unsigned char c[64]; } bar (void);
+vector unsigned char v;
+
+void
+foo (void)
+{
+ vec_ld (0, bar ().c); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */
+ vec_st (v, 0, bar ().c); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */
+}
--- gcc/testsuite/g++.dg/ext/altivec-18.C.jj 2017-09-08 09:15:20.593774717 +0200
+++ gcc/testsuite/g++.dg/ext/altivec-18.C 2017-09-08 09:15:20.593774717 +0200
@@ -0,0 +1,14 @@
+// PR target/82112
+// { dg-do compile { target powerpc*-*-* } }
+// { dg-require-effective-target powerpc_altivec_ok }
+// { dg-options "-save-temps -maltivec" }
+
+#include <altivec.h>
+
+__attribute__((aligned (16))) extern const unsigned char c[16];
+
+void
+foo (void)
+{
+ vec_ld (0, c);
+}
Jakub