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 02C41385C403 for ; Tue, 20 Jul 2021 00:05:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 02C41385C403 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=fail 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 16K04Jlf028732; Mon, 19 Jul 2021 19:04:19 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 16K04IHg028731; Mon, 19 Jul 2021 19:04:18 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 19 Jul 2021 19:04:18 -0500 From: Segher Boessenkool To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 15/55] rs6000: Build and store function type identifiers Message-ID: <20210720000418.GU1583@gate.crashing.org> References: <7931e390f98572d6c3512d9da8f1af1c1f74418e.1623941441.git.wschmidt@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7931e390f98572d6c3512d9da8f1af1c1f74418e.1623941441.git.wschmidt@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, TXREP, T_SPF_HELO_PERMERROR, T_SPF_PERMERROR 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, 20 Jul 2021 00:05:21 -0000 Hi! On Thu, Jun 17, 2021 at 10:18:59AM -0500, Bill Schmidt wrote: > * config/rs6000/rs6000-gen-builtins.c (complete_vector_type): New > function. > (complete_base_type): Likewise. > (construct_fntype_id): Likewise. > (parse_bif_entry): Call contruct_fntype_id. > (parse_ovld_entry): Likewise. > +/* Convert a vector type into a mode string. */ > +static void > +complete_vector_type (typeinfo *typeptr, char *buf, int *bufi) > +{ > + if (typeptr->isbool) > + buf[(*bufi)++] = 'b'; > + buf[(*bufi)++] = 'v'; > + if (typeptr->ispixel) > + { > + memcpy (&buf[*bufi], "p8hi", 4); > + *bufi += 4; return; } > + else ... and then you don't need this, and everything after this loses a level of indentation. The power of early outs :-) > +/* Build a function type descriptor identifier from the return type > + and argument types described by PROTOPTR, and store it if it does > + not already exist. Return the identifier. */ > +static char * > +construct_fntype_id (prototype *protoptr) > +{ > + /* Determine the maximum space for a function type descriptor id. > + Each type requires at most 9 characters (6 for the mode*, 1 for > + the optional 'u' preceding the mode, 1 for the optional 'p' > + preceding the mode, and 1 for an underscore following the mode). > + We also need 5 characters for the string "ftype" that separates > + the return mode from the argument modes. The last argument doesn't > + need a trailing underscore, but we count that as the one trailing > + "ftype" instead. For the special case of zero arguments, we need 9 > + for the return type and 7 for "ftype_v". Finally, we need one > + character for the terminating null. Thus for a function with N > + arguments, we need at most 9N+15 characters for N>0, otherwise 17. > + ---- > + *Worst case is bv16qi for "vector bool char". */ > + int len = protoptr->nargs ? (protoptr->nargs + 1) * 9 + 6 : 17; > + char *buf = (char *) malloc (len); I would completely avoid all this by using asprintf. But that requires a little restructuring, so *shrug* (it could use some factoring anyway, but we don't need this to be perfect ;-) ) > + assert (!argptr); > + } Formatting here is broken... Probably that last line has two spaces to many, but please check. > + /* Ignore return value, as duplicates are expected. */ > + (void) rbt_insert (&fntype_rbt, buf); Casting to void does not ignore a return value. But is rbt_insert marked up as warn_unused_result anyway? Simply not using the return value is fine as well. If you want to be very explicit, you can write if (rbt_insert (&fntype_rbt, buf)) ; /* Duplicates are fine and expected here. */ Okay for trunk with or without such improvements. But do fix that indentation thing please ;-) Thanks! Segher