Re: [PATCH, rs6000] PR84220 fix altivec_vec_sld and vec_sldw intrinsic definitions

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi!

On Wed, Feb 07, 2018 at 09:14:59AM -0600, Will Schmidt wrote:
>   Our VEC_SLD definitions were mistakenly allowing the third argument to be
> of an invalid type, triggering an ICE (on invalid code) later in the build
> process.  This fixes those definitions.  The nearby VEC_SLDW definitions have
> the same issue, those have been fixed as part of this patch too.
> Testcases have been added to ensure we generate the 'invalid intrinsic'
> message as is appropriate, instead of ICEing.
> Giving proper credit, this was found by Peter Bergner while working a
> different issue. :-)
> 
> Sniff-tests passed on P8.  Doing larger reg-test across power systems now.
> OK for trunk?
> And,.. do we want this one backported too?

> diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
> index a68be51..26f9990 100644
> --- a/gcc/config/rs6000/rs6000-c.c
> +++ b/gcc/config/rs6000/rs6000-c.c
> @@ -3654,39 +3654,39 @@ 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 },

It isn't clear to me what RS6000_BTI_NOT_OPAQUE means...  rs6000-c.c says:

        /* For arguments after the last, we have RS6000_BTI_NOT_OPAQUE in
           the opX fields.  */

(whatever that means!), and the following code seems to allow anything in
such args?  If you understand it, please update some comments somewhere?

>    { VSX_BUILTIN_VEC_XXPERMDI, VSX_BUILTIN_XXPERMDI_2DF,

XXPERMDI is the only other builtin that uses NOT_OPAQUE, does that suffer
from the same problem?  If so, you can completely delete NOT_OPAQUE it
seems?

So what is/was it for, that is what I wonder.

Your patch looks fine if you can clear that up :-)


Segher