Hi Jakub,
On Tue, Sep 12, 2017 at 04:25:48PM +0200, Jakub Jelinek wrote:
> 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.
>
> 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?
It looks fine to me; Bill, could you take a look? You know this stuff
much better than I do :-)
One question below:
> 2017-09-12 Jakub Jelinek <ja...@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" }
What is this -save-temps for? Just a leftover?
> +#include <altivec.h>
> +
> +__attribute__((aligned (16))) extern const unsigned char c[16];
> +
> +void
> +foo (void)
> +{
> + vec_ld (0, c);
> +}
So, okay for trunk as far as I can see. Thanks,
Segher
