Re: [PATCH, rs6000] (v2) PR84220 remove RS6000_BTI_NOT_OPAQUE refs from builtins table

2018-02-14 Thread Will Schmidt 
On Wed, 2018-02-14 at 04:53 -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Feb 13, 2018 at 05:40:08PM -0600, Will Schmidt wrote:
> >   Some of our builtin definitions were allowing invalid parameters, and a
> > subsequent ICE (on invalid code) were the result.  This is due to the use of
> > RS6000_BTI_NOT_OPAQUE (which allowed vector arguments), where a
> > RS6000_BTI_INTSI appears to be a more appropriate choice.
> > This change adjusts the definitions for the VEC_SLD, VEC_SLDW, vec_XXSLDWI
> > and VEC_XXPERMDI entries.
> 
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr84220-xxperm.c
> > @@ -0,0 +1,100 @@
> > +/* PR target/84220 */
> > +/* Test to ensure we generate invalid parameter errors rather than an ICE
> > +when calling vec_xxpermdi() with invalid parameters.  */
> > +/* { dg-do compile { target { powerpc64*-*-* } } } */
> > +/* { dg-require-effective-target powerpc_vsx_ok } */
> > +/* { dg-options "-O2 -mvsx" } */
> 
> Does this test need powerpc64*?  Or does it need lp64 instead, or nothing?

Good catch.  :-)   slightly more coverage when removing the target.
Changed it to just a dg-do compile stanza to match the other tests.

Thanks,
-Will

> Looks good, please look at that detail again; okay for trunk.  Thanks!
> 
> 
> Segher
>

Re: [PATCH, rs6000] (v2) PR84220 remove RS6000_BTI_NOT_OPAQUE refs from builtins table

2018-02-14 Thread Segher Boessenkool 
Hi!

On Tue, Feb 13, 2018 at 05:40:08PM -0600, Will Schmidt wrote:
>   Some of our builtin definitions were allowing invalid parameters, and a
> subsequent ICE (on invalid code) were the result.  This is due to the use of
> RS6000_BTI_NOT_OPAQUE (which allowed vector arguments), where a
> RS6000_BTI_INTSI appears to be a more appropriate choice.
> This change adjusts the definitions for the VEC_SLD, VEC_SLDW, vec_XXSLDWI
> and VEC_XXPERMDI entries.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr84220-xxperm.c
> @@ -0,0 +1,100 @@
> +/* PR target/84220 */
> +/* Test to ensure we generate invalid parameter errors rather than an ICE
> +when calling vec_xxpermdi() with invalid parameters.  */
> +/* { dg-do compile { target { powerpc64*-*-* } } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -mvsx" } */

Does this test need powerpc64*?  Or does it need lp64 instead, or nothing?

Looks good, please look at that detail again; okay for trunk.  Thanks!


Segher

[PATCH, rs6000] (v2) PR84220 remove RS6000_BTI_NOT_OPAQUE refs from builtins table

2018-02-13 Thread Will Schmidt 
Hi,
This is (v2), which is notably more comprehensive than (v1).
  Some of our builtin definitions were allowing invalid parameters, and a
subsequent ICE (on invalid code) were the result.  This is due to the use of
RS6000_BTI_NOT_OPAQUE (which allowed vector arguments), where a
RS6000_BTI_INTSI appears to be a more appropriate choice.
This change adjusts the definitions for the VEC_SLD, VEC_SLDW, vec_XXSLDWI
and VEC_XXPERMDI entries.

Testcases have been added to ensure we generate the 'invalid intrinsic'
message as is appropriate, instead of ICEing.
Giving credit, this was found by Peter Bergner while working another issue.

Sniff-tests passed on P8.  Doing larger reg-test across power systems now.
OK for trunk?
Thanks,
-Will

[gcc]

2018-02-13  Will Schmidt  

PR target/84220
* config/rs6000/rs6000-c.c: Update definitions for
ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VEC_SLDW,
VSX_BUILTIN_VEC_XXSLDWI, and ALTIVEC_BUILTIN_VEC_XXPERMDI builtins.

[testsuite]

2018-02-13  Will Schmidt  

PR target/84220
* gcc.target/powerpc/pr84220-sld.c: New test.
* gcc.target/powerpc/pr84220-sld2.c: New test.
* gcc.target/powerpc/pr84220-sldw.c: New test.
* gcc.target/powerpc/pr84220-xxperm.c: New test.
* gcc.target/powerpc/pr84220-xxsld.c: New test.

diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index a68be51..843a375 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -3654,64 +3654,65 @@ const struct altivec_builtin_types 
altivec_overloaded_builtins[] = {
   { ALTIVEC_BUILTIN_VEC_SEL, ALTIVEC_BUILTIN_VSEL_16QI,
 RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, 
RS6000_BTI_bool_V16QI },
   { ALTIVEC_BUILTIN_VEC_SEL, ALTIVEC_BUILTIN_VSEL_16QI,
 RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, 
RS6000_BTI_unsigned_V16QI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_4SF,
-RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_4SI,
-RS6000_BTI_V4SI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_V4SI, RS6000_BTI_V4SI, RS6000_BTI_V4SI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_4SI,
-RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V4SI, 
RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V4SI, 
RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_4SI,
-RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, 
RS6000_BTI_unsigned_V4SI, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, 
RS6000_BTI_unsigned_V4SI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_8HI,
-RS6000_BTI_V8HI, RS6000_BTI_V8HI, RS6000_BTI_V8HI, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_V8HI, RS6000_BTI_V8HI, RS6000_BTI_V8HI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_8HI,
-RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, 
RS6000_BTI_unsigned_V8HI, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, 
RS6000_BTI_unsigned_V8HI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_8HI,
-RS6000_BTI_bool_V8HI, RS6000_BTI_bool_V8HI, RS6000_BTI_bool_V8HI, 
RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_bool_V8HI, RS6000_BTI_bool_V8HI, RS6000_BTI_bool_V8HI, 
RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_8HI,
-RS6000_BTI_pixel_V8HI, RS6000_BTI_pixel_V8HI, RS6000_BTI_pixel_V8HI, 
RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_pixel_V8HI, RS6000_BTI_pixel_V8HI, RS6000_BTI_pixel_V8HI, 
RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_16QI,
-RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, 
RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_16QI,
-RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 
RS6000_BTI_unsigned_V16QI, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 
RS6000_BTI_unsigned_V16QI, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_16QI,
-RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, 
RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, 
RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_2DF,
-RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_NOT_OPAQUE },
+RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_INTSI },
   { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_2DI,
-RS6000_BTI_bool_V2DI,