From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id C04553858420 for ; Fri, 28 Jan 2022 21:19:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C04553858420 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 20SL8Y0r030297; Fri, 28 Jan 2022 21:19:52 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dvpeq24p5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 28 Jan 2022 21:19:52 +0000 Received: from m0098393.ppops.net (m0098393.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 20SLDIs9012119; Fri, 28 Jan 2022 21:19:51 GMT Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dvpeq24p2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 28 Jan 2022 21:19:51 +0000 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 20SL82Fg027173; Fri, 28 Jan 2022 21:19:51 GMT Received: from b01cxnp23033.gho.pok.ibm.com (b01cxnp23033.gho.pok.ibm.com [9.57.198.28]) by ppma03dal.us.ibm.com with ESMTP id 3dr9jds1bg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 28 Jan 2022 21:19:50 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 20SLJnYu41353572 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 28 Jan 2022 21:19:49 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5E86F124053; Fri, 28 Jan 2022 21:19:49 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F40C312405B; Fri, 28 Jan 2022 21:19:48 +0000 (GMT) Received: from [9.211.95.53] (unknown [9.211.95.53]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP; Fri, 28 Jan 2022 21:19:48 +0000 (GMT) Message-ID: <4b46e4f0-f186-3796-a4df-14fd57fea33a@linux.ibm.com> Date: Fri, 28 Jan 2022 15:19:48 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 Reply-To: wschmidt@linux.ibm.com Subject: Re: [PATCH 1/8] rs6000: More factoring of overload processing To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com References: <9ee506e947ec49973f757ea4a967574ded4ed2b0.1643390744.git.wschmidt@linux.ibm.com> <20220128191110.GG614@gate.crashing.org> From: Bill Schmidt In-Reply-To: <20220128191110.GG614@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: OijmF36BiZR7giwnujkY7iw8eaOFk7zA X-Proofpoint-ORIG-GUID: SS2iQ8yIFcB0zsOtG_JhInWVaXH7bQrp X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-01-28_07,2022-01-28_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 clxscore=1015 spamscore=0 adultscore=0 suspectscore=0 mlxlogscore=999 impostorscore=0 lowpriorityscore=0 priorityscore=1501 bulkscore=0 mlxscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000 definitions=main-2201280120 X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Jan 2022 21:19:56 -0000 On 1/28/22 1:11 PM, Segher Boessenkool wrote: > On Fri, Jan 28, 2022 at 11:50:19AM -0600, Bill Schmidt wrote: >> This patch continues the refactoring started with r12-6014. > ab3f5b71dc6e > >> + and the generic code will issue the appropriate error message. Skip >> + this test for functions where we don't fully describe all the possible >> + overload signatures in rs6000-overload.def (because they aren't relevant >> + to the expansion here). If we don't, we get confusing error messages. */ >> + if (fcode != RS6000_OVLD_VEC_PROMOTE >> + && fcode != RS6000_OVLD_VEC_SPLATS >> + && fcode != RS6000_OVLD_VEC_EXTRACT >> + && fcode != RS6000_OVLD_VEC_INSERT >> + && fcode != RS6000_OVLD_VEC_STEP >> + && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs)) >> return NULL; > Can you expand a bit on this, give an example for example? It is very > hard to understand this code, the way it depends on code following many > lines later. Sure, sorry. This check gives up if the number of arguments doesn't match the prototype. It gives a fairly generic error message. That part of it has always been in here. Now, I moved this check forward relative to the big switch statement on fcode, because there are redundant checks for the number of arguments in each of the resolve_vec_* helper functions. This allowed me to simplify those a bit. Now, it turns out that this doesn't work so well for functions that aren't fully described in rs6000-overload.def. For example, for vec_splats we have: ; There are no actual builtins for vec_splats. There is special handling for ; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call ; is replaced by a constructor. The single overload here causes ; __builtin_vec_splats to be registered with the front end so that can happen. [VEC_SPLATS, vec_splats, __builtin_vec_splats] vsi __builtin_vec_splats (vsi); ABS_V4SI SPLATS_FAKERY So even though __builtin_vec_splats accepts all vector types, the infrastructure cheats and just records one prototype. We end up getting an error message that refers to this specific prototype even when we are handling a different argument type. That is completely confusing to the user. So I felt I was starting to get too deep for a simple refactoring patch, and gave up on early number-of-arguments checking for the special cases that use the _FAKERY technique. That's probably still not clear, but maybe clearer? > >> + default: >> + ; > Don't. > > I like this better than a BS break statement, but it is just as stupid. > > If you need this, you don't want a switch statement, but some number of > if statements. You cannot use a switch as a shorthand for this because > we have a silly warning and -Werror for this use. > > You probably get easier to understand code that way, too, you can get > rid of the above (just do some early returns), etc. If I understand correctly, you'd like me to resubmit this in if-then-else form. That's fine, just want to be sure that's what you want. Thanks for the review! Bill > > > Segher