| Message ID | 1518016499.11602.261.camel@brimstone.rchland.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [rs6000] PR84220 fix altivec_vec_sld and vec_sldw intrinsic definitions | expand |
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
On Thu, 2018-02-08 at 17:48 -0600, Segher Boessenkool wrote: > 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? I do not... only got as far as figuring out it wasn't right for vec_sld*(). I'll poke around a bit to see what I can figure out. May need to punt to (???) to understand the intent. Mike?/Peter? A bit more context around that usage is: [rs6000-c.c: altivec_resolve_overloaded_builtin() ] ... /* For arguments after the last, we have RS6000_BTI_NOT_OPAQUE in the opX fields. */ for (; desc->code == fcode; desc++) { if ((desc->op1 == RS6000_BTI_NOT_OPAQUE || rs6000_builtin_type_compatible (types[0], desc->op1)) ... with that check repeated against types[1], desc->op2,.. etc. > > { 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? Not sure. > So what is/was it for, that is what I wonder. > > Your patch looks fine if you can clear that up :-) Heh, Ok. > > > Segher >
On Thu, 2018-02-08 at 17:48 -0600, Segher Boessenkool wrote: > 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? I dug in a bit more to try to understand the history and context. The RS6000_BTI_NOT_OPAQUE entry was added as part of the (large) AltiVec rewrite (By Paolo Bonzini) in Apr 2005. The ALTIVEC_BUILTIN_VEC_SLD entries, and the "for arguments after the last" code chunk in altivec_resolve_overloaded_builtin() was part of that addition, and pretty much un-touched since that time. > > { 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? Yes and no. I've generated a few more tests that show the problem also included vec_xxperms. SO,.. I've updated the patch to fix those references too. With that change, all references to NOT_OPAQUE in the builtins table are removed. (I'll be posting that momentarily while regtests run overnight..) So then with the idea of cleaning up all remaining references to _NOT_OPAQUE.. I got stuck. :-) The _NOT_OPAQUE definition is the first entry in the (rs6000.h: enum rs6000_builtin_type_index) enum rs6000_builtin_type_index { - RS6000_BTI_NOT_OPAQUE, + RS6000_BTI_unset, RS6000_BTI_opaque_V2SI, And the only other reference is in this chunk of code in rs6000-c.c: altivec_resolve_overloaded_builtin() /* For arguments after the last, we have RS6000_BTI_NOT_OPAQUE in the opX fields. */ for (; desc->code == fcode; desc++) { if ((desc->op1 == RS6000_BTI_NOT_OPAQUE || rs6000_builtin_type_compatible (types[0], desc->op1)) && (desc->op2 == RS6000_BTI_NOT_OPAQUE || rs6000_builtin_type_compatible (types[1], desc->op2)) && (desc->op3 == RS6000_BTI_NOT_OPAQUE || rs6000_builtin_type_compatible (types[2], desc->op3))) So there should no longer be any matches to ...NOT_OPAQUE, but if I comment out the snippets "== ..NOT_OPAQUE || ", lots of ICE's show up. which makes me wonder if the check here is more of a "if desc->op1 was not explicitly set,... " thing. But it's not clear to me. So I'm deliberately not touching this chunk of code at this time. :-) Thanks -Will > > So what is/was it for, that is what I wonder. > > Your patch looks fine if you can clear that up :-) > > > Segher >
> On Feb 13, 2018, at 5:28 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > > On Thu, 2018-02-08 at 17:48 -0600, Segher Boessenkool wrote: >> 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? The intent is that each entry in the table has a maximum of one result type and three argument types. If a built-in only takes one argument, then the second and third argument entries in the table should be flagged as RS6000_BTI_NOT_OPAQUE. I have never understood why that particular (misleading) term was used. But basically it means (as Will found in the code chunk below) that you shouldn't check type compatibility because we don't expect an argument in that position. At least that's always been my understanding. I could be wrong. It seems very odd that you are finding it easy to remove this (only a few entries), so I might be way off base. Bill > > I dug in a bit more to try to understand the history and context. > > The RS6000_BTI_NOT_OPAQUE entry was added as part of the (large) AltiVec > rewrite (By Paolo Bonzini) in Apr 2005. > The ALTIVEC_BUILTIN_VEC_SLD entries, and the "for arguments after the > last" code chunk in altivec_resolve_overloaded_builtin() was part of > that addition, and pretty much un-touched since that time. > >>> { 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? > > Yes and no. I've generated a few more tests that show the problem also > included vec_xxperms. > SO,.. I've updated the patch to fix those references too. > > With that change, all references to NOT_OPAQUE in the builtins table are > removed. > > (I'll be posting that momentarily while regtests run overnight..) > > So then with the idea of cleaning up all remaining references to > _NOT_OPAQUE.. I got stuck. :-) > > The _NOT_OPAQUE definition is the first entry in the (rs6000.h: enum > rs6000_builtin_type_index) > > enum rs6000_builtin_type_index > { > - RS6000_BTI_NOT_OPAQUE, > + RS6000_BTI_unset, > RS6000_BTI_opaque_V2SI, > > > And the only other reference is in this chunk of code in rs6000-c.c: > altivec_resolve_overloaded_builtin() > > /* For arguments after the last, we have RS6000_BTI_NOT_OPAQUE in > the opX fields. */ > for (; desc->code == fcode; desc++) > { > if ((desc->op1 == RS6000_BTI_NOT_OPAQUE > || rs6000_builtin_type_compatible (types[0], desc->op1)) > && (desc->op2 == RS6000_BTI_NOT_OPAQUE > || rs6000_builtin_type_compatible (types[1], desc->op2)) > && (desc->op3 == RS6000_BTI_NOT_OPAQUE > || rs6000_builtin_type_compatible (types[2], desc->op3))) > > > So there should no longer be any matches to ...NOT_OPAQUE, but if I > comment out the snippets "== ..NOT_OPAQUE || ", lots of ICE's show up. > > which makes me wonder if the check here is more of a "if desc->op1 was > not explicitly set,... " thing. But it's not clear to me. > > So I'm deliberately not touching this chunk of code at this time. :-) > > Thanks > -Will > > >> >> So what is/was it for, that is what I wonder. >> >> Your patch looks fine if you can clear that up :-) >> >> >> Segher >> > >
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 }, { 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, RS6000_BTI_bool_V2DI, RS6000_BTI_bool_V2DI, RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_bool_V2DI, RS6000_BTI_bool_V2DI, RS6000_BTI_bool_V2DI, RS6000_BTI_INTSI }, { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_2DI, - RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_INTSI }, { ALTIVEC_BUILTIN_VEC_SLD, ALTIVEC_BUILTIN_VSLDOI_2DI, - RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI }, { ALTIVEC_BUILTIN_VEC_SLDW, VSX_BUILTIN_XXSLDWI_16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_NOT_OPAQUE }, { ALTIVEC_BUILTIN_VEC_SLDW, VSX_BUILTIN_XXSLDWI_16QI, @@ -4152,29 +4152,29 @@ const struct altivec_builtin_types altivec_overloaded_builtins[] = { ~RS6000_BTI_unsigned_V16QI }, { VSX_BUILTIN_VEC_XST_BE, VSX_BUILTIN_ST_ELEMREV_V16QI, RS6000_BTI_void, RS6000_BTI_unsigned_V16QI, RS6000_BTI_INTSI, ~RS6000_BTI_UINTQI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_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 }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_16QI, RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, - RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_INTSI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_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 }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_8HI, RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, - RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_INTSI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_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 }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_4SI, RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, - RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_INTSI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_2DI, - RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_V2DI, RS6000_BTI_INTSI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, - RS6000_BTI_NOT_OPAQUE }, + RS6000_BTI_INTSI }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_4SF, RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_V4SF, RS6000_BTI_NOT_OPAQUE }, { VSX_BUILTIN_VEC_XXSLDWI, VSX_BUILTIN_XXSLDWI_2DF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, RS6000_BTI_NOT_OPAQUE }, { VSX_BUILTIN_VEC_XXPERMDI, VSX_BUILTIN_XXPERMDI_2DF, diff --git a/gcc/testsuite/gcc.target/powerpc/pr84220-1.c b/gcc/testsuite/gcc.target/powerpc/pr84220-1.c new file mode 100644 index 0000000..f2454d1 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr84220-1.c @@ -0,0 +1,95 @@ +/* PR target/84220 */ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-options "-maltivec" } */ + +#include <altivec.h> + +typedef vector bool char vbc_t; +typedef vector signed char vsc_t; +typedef vector unsigned char vuc_t; +typedef vector bool int vbi_t; +typedef vector signed int vsi_t; +typedef vector unsigned int vui_t; +typedef vector pixel vp_t; +typedef vector bool short vbs_t; +typedef vector signed short vss_t; +typedef vector unsigned short vus_t; +typedef vector float vf_t; + +void +test_vbc ( vbc_t v1, vbc_t v2, vbc_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vsc ( vsc_t v1, vsc_t v2, vsc_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vuc ( vuc_t v1, vuc_t v2, vuc_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vbi ( vbi_t v1, vbi_t v2, vbi_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vsi ( vsi_t v1, vsi_t v2, vsi_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vui ( vui_t v1, vui_t v2, vui_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vp ( vp_t v1, vp_t v2, vp_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vbs ( vbs_t v1, vbs_t v2, vbs_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vss ( vss_t v1, vss_t v2, vss_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vus ( vus_t v1, vus_t v2, vus_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vf ( vf_t v1, vf_t v2, vf_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} diff --git a/gcc/testsuite/gcc.target/powerpc/pr84220-2.c b/gcc/testsuite/gcc.target/powerpc/pr84220-2.c new file mode 100644 index 0000000..1e8a965 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr84220-2.c @@ -0,0 +1,40 @@ +/* PR target/84220 */ +/* Same as pr84220-1.c , with p8vector requirement for long long and double types. */ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_p8vector_ok } */ +/* { dg-options "-maltivec -mpower8-vector" } */ + +#include <altivec.h> + +typedef vector bool long long vbl_t; +typedef vector signed long long vsl_t; +typedef vector unsigned long long vul_t; +typedef vector double vd_t; + +void +test_vbl ( vbl_t v1, vbl_t v2, vbl_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vsl ( vsl_t v1, vsl_t v2, vsl_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vul ( vul_t v1, vul_t v2, vul_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} + +void +test_vd ( vd_t v1, vd_t v2, vd_t v3 ) \ +{ + __builtin_vec_sld(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + __builtin_vec_sld(v1, v2, 3); +} diff --git a/gcc/testsuite/gcc.target/powerpc/pr84220-sldw.c b/gcc/testsuite/gcc.target/powerpc/pr84220-sldw.c new file mode 100644 index 0000000..0f50837 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr84220-sldw.c @@ -0,0 +1,78 @@ +/* PR target/84220 */ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-mvsx" } */ + +#include <altivec.h> + +typedef vector bool char vbc_t; +typedef vector signed char vsc_t; +typedef vector unsigned char vuc_t; +typedef vector bool int vbi_t; +typedef vector signed int vsi_t; +typedef vector unsigned int vui_t; +typedef vector pixel vp_t; +typedef vector bool short vbs_t; +typedef vector signed short vss_t; +typedef vector unsigned short vus_t; +typedef vector float vf_t; +typedef vector bool long long vbl_t; +typedef vector signed long long vsl_t; +typedef vector unsigned long long vul_t; +typedef vector double vd_t; + +void +test_vsc ( vsc_t v1, vsc_t v2, vsc_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vuc ( vuc_t v1, vuc_t v2, vuc_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vsi ( vsi_t v1, vsi_t v2, vsi_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vui ( vui_t v1, vui_t v2, vui_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vsl ( vsl_t v1, vsl_t v2, vsl_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vul ( vul_t v1, vul_t v2, vul_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vss ( vss_t v1, vss_t v2, vss_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +} + +void +test_vus ( vus_t v1, vus_t v2, vus_t v3 ) \ +{ + vec_sldw(v1, v2, v3); /* { dg-error "invalid parameter combination for AltiVec intrinsic" } */ + vec_sldw(v1, v2, 3); +}
Hi, 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? Thanks, -Will [gcc] 2018-02-07 Will Schmidt <will_schmidt@vnet.ibm.com> PR target/84220 * config/rs6000/rs6000-c.c: Update definitions for ALTIVEC_BUILTIN_VEC_SLD and ALTIVEC_BUILTIN_VEC_SLDW. [testsuite] 2018-02-07 Will Schmidt <will_schmidt@vnet.ibm.com> PR target/84220 * gcc.target/powerpc/pr84220-1.c: New test. * gcc.target/powerpc/pr84220-2.c: New test. * gcc.target/powerpc/pr84220-sldw.c: New test.