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 CA98C385840D for ; Sun, 8 Aug 2021 20:28:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CA98C385840D 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 178KRBN6022232; Sun, 8 Aug 2021 15:27:11 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 178KRAMO022231; Sun, 8 Aug 2021 15:27:10 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Sun, 8 Aug 2021 15:27:10 -0500 From: Segher Boessenkool To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, willschm@linux.ibm.com Subject: Re: [PATCH 03/34] rs6000: Add the rest of the [altivec] stanza to the builtins file Message-ID: <20210808202710.GC1583@gate.crashing.org> References: <5394facbf34d21fab7944808ccb27dca74f6f51f.1627562851.git.wschmidt@linux.ibm.com> <20210807000158.GZ1583@gate.crashing.org> <9842d456-9003-8cbf-0e13-40821ae4217c@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9842d456-9003-8cbf-0e13-40821ae4217c@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-5.8 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: Sun, 08 Aug 2021 20:28:13 -0000 Hi! On Sun, Aug 08, 2021 at 11:53:38AM -0500, Bill Schmidt wrote: > On 8/6/21 7:01 PM, Segher Boessenkool wrote: > >On Thu, Jul 29, 2021 at 08:30:50AM -0500, Bill Schmidt wrote: > >>+ const vsc __builtin_altivec_abss_v16qi (vsc); > >>+ ABSS_V16QI altivec_abss_v16qi {} > >>+ > >>+ const vsi __builtin_altivec_abss_v4si (vsi); > >>+ ABSS_V4SI altivec_abss_v4si {} > >>+ > >>+ const vss __builtin_altivec_abss_v8hi (vss); > >>+ ABSS_V8HI altivec_abss_v8hi {} > >Is there any ordering used here? What is it, then? Just alphabetical? > > > >That order does not really allow breaking things up into groups, which > >is the main tool to keep things manageable. > > Yes, within each stanza, the ordering is alphabetical by built-in name.  > It seems to me that any other ordering is arbitrary and prone to > requiring exceptions, so in the end you just end up with a mess where > nobody knows where to put the next builtin added. That's certainly what > happened with the old support. Yeah, there is no great answer here :-( You have thought about it in any case, so let's see where this goes. > >>+ const vsc __builtin_vec_init_v16qi (signed char, signed char, signed > >>char, signed char, signed char, signed char, signed char, signed char, > >>signed char, signed char, signed char, signed char, signed char, signed > >>char, signed char, signed char); > >That is a very long line, can you do something about that, or is that > >forced by the file format? Can you use just "char"? "signed char" is a > >very strange choice. > > Right now, long lines are there because the parser doesn't support > breaking up the line.  I have an additional patch I put together > recently that allows the use of escape-newline to break up these lines.  > I am planning to submit that once we get through the current patch set. Okido. What about the signed char though? > >>+ pcvoid_type_node > >>+ = build_pointer_type (build_qualified_type (void_type_node, > >>+ TYPE_QUAL_CONST)); > >A const void? Interesting. You are building a pointer to a const void > >here, not a const pointer to void. Is that what you wanted? > > > >(And yes I do realise this is just moved, not new code). > > > Sorry, I misdocumented this below.  I'll review and make sure this is > correct everywhere. "const void" is meaningless, and maybe even invalid C. I think the code is wrong, not (just) the documentation! This wants to be void *const but it is const void * as far as I can see? As I said, this isn't new code, but it seems very wrong! Segher