From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 634563858C83 for ; Tue, 1 Feb 2022 21:49:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 634563858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 211LmaqV014120; Tue, 1 Feb 2022 15:48:36 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 211Lmakh014119; Tue, 1 Feb 2022 15:48:36 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 1 Feb 2022 15:48:36 -0600 From: Segher Boessenkool To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com Subject: Re: [PATCH v2 1/8] rs6000: More factoring of overload processing Message-ID: <20220201214836.GV614@gate.crashing.org> References: <9ee506e947ec49973f757ea4a967574ded4ed2b0.1643390744.git.wschmidt@linux.ibm.com> <20220128191110.GG614@gate.crashing.org> <1d7fa906-bf3a-36bc-eeee-3c1eebb078e1@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1d7fa906-bf3a-36bc-eeee-3c1eebb078e1@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no 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: Tue, 01 Feb 2022 21:49:39 -0000 On Tue, Feb 01, 2022 at 08:49:34AM -0600, Bill Schmidt wrote: > I've modified the previous patch to add more explanatory commentary about > the number-of-arguments test that was previously confusing, and to convert > the switch into an if-then-else chain. The rest of the patch is unchanged. > Bootstrapped and tested on powerpc64le-linux-gnu. Is this okay for trunk? > gcc/ > * config/rs6000/rs6000-c.cc (resolve_vec_mul): Accept args and types > parameters instead of arglist and nargs. Simplify accordingly. Remove > unnecessary test for argument count mismatch. > (resolve_vec_cmpne): Likewise. > (resolve_vec_adde_sube): Likewise. > (resolve_vec_addec_subec): Likewise. > (altivec_resolve_overloaded_builtin): Move overload special handling > after the gathering of arguments into args[] and types[] and the test > for correct number of arguments. Don't perform the test for correct > number of arguments for certain special cases. Call the other special > cases with args and types instead of arglist and nargs. > + 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; Please don't do De Morgan manually, let the compiler deal with it? Although even with that the logic is as clear as mud. This matters if someone (maybe even you) will have to debug this later, or modify this. Maybe adding some suitably named variables can clarify things here? > + if (fcode == RS6000_OVLD_VEC_MUL) > + returned_expr = resolve_vec_mul (&res, args, types, loc); > + else if (fcode == RS6000_OVLD_VEC_CMPNE) > + returned_expr = resolve_vec_cmpne (&res, args, types, loc); > + else if (fcode == RS6000_OVLD_VEC_ADDE || fcode == RS6000_OVLD_VEC_SUBE) > + returned_expr = resolve_vec_adde_sube (&res, fcode, args, types, loc); > + else if (fcode == RS6000_OVLD_VEC_ADDEC || fcode == RS6000_OVLD_VEC_SUBEC) > + returned_expr = resolve_vec_addec_subec (&res, fcode, args, types, loc); > + else if (fcode == RS6000_OVLD_VEC_SPLATS || fcode == RS6000_OVLD_VEC_PROMOTE) > + returned_expr = resolve_vec_splats (&res, fcode, arglist, nargs); > + else if (fcode == RS6000_OVLD_VEC_EXTRACT) > + returned_expr = resolve_vec_extract (&res, arglist, nargs, loc); > + else if (fcode == RS6000_OVLD_VEC_INSERT) > + returned_expr = resolve_vec_insert (&res, arglist, nargs, loc); > + else if (fcode == RS6000_OVLD_VEC_STEP) > + returned_expr = resolve_vec_step (&res, arglist, nargs); > + > + if (res == resolved) > + return returned_expr; This is so convoluted because the functions do two things, and have two return values (res and returned_expr). Segher