From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3965 invoked by alias); 13 Feb 2018 23:29:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 3955 invoked by uid 89); 13 Feb 2018 23:29:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Feb 2018 23:29:15 +0000 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w1DNTD2c031292 for ; Tue, 13 Feb 2018 18:29:13 -0500 Received: from e16.ny.us.ibm.com (e16.ny.us.ibm.com [129.33.205.206]) by mx0b-001b2d01.pphosted.com with ESMTP id 2g45utyw8c-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 13 Feb 2018 18:29:12 -0500 Received: from localhost by e16.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 13 Feb 2018 18:29:02 -0500 Received: from b01cxnp22036.gho.pok.ibm.com (9.57.198.26) by e16.ny.us.ibm.com (146.89.104.203) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 13 Feb 2018 18:28:59 -0500 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w1DNSwpt48234596; Tue, 13 Feb 2018 23:28:58 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 93F67B2050; Tue, 13 Feb 2018 18:25:52 -0500 (EST) Received: from [9.10.86.107] (unknown [9.10.86.107]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id 4E2ACB204D; Tue, 13 Feb 2018 18:25:52 -0500 (EST) Subject: Re: [PATCH, rs6000] PR84220 fix altivec_vec_sld and vec_sldw intrinsic definitions From: Will Schmidt Reply-To: will_schmidt@vnet.ibm.com To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, David Edelsohn , Bill Schmidt , Peter Bergner In-Reply-To: <20180208234826.GM21977@gate.crashing.org> References: <1518016499.11602.261.camel@brimstone.rchland.ibm.com> <20180208234826.GM21977@gate.crashing.org> Content-Type: text/plain; charset="UTF-8" Date: Tue, 13 Feb 2018 23:29:00 -0000 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 18021323-0024-0000-0000-00000323040A X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008529; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000253; SDB=6.00989328; UDB=6.00502344; IPR=6.00768683; BA=6.00005827; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00019539; XFM=3.00000015; UTC=2018-02-13 23:29:00 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18021323-0025-0000-0000-000046F849BA Message-Id: <1518564537.11602.327.camel@brimstone.rchland.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2018-02-13_11:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1802130274 X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00796.txt.bz2 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 >