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 619D43858427 for ; Mon, 1 Nov 2021 12:19:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 619D43858427 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 1A1CI5lU020700; Mon, 1 Nov 2021 07:18:05 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 1A1CI5ff020699; Mon, 1 Nov 2021 07:18:05 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 1 Nov 2021 07:18:04 -0500 From: Segher Boessenkool To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com Subject: Re: [PATCH 07/18] rs6000: Builtin expansion, part 2 Message-ID: <20211101121804.GE614@gate.crashing.org> References: <2d37e1858ac2d6e10a8f167d92fad83c31077af2.1630511335.git.wschmidt@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2d37e1858ac2d6e10a8f167d92fad83c31077af2.1630511335.git.wschmidt@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_NUMSUBJECT, SPF_HELO_PASS, SPF_PASS, TXREP 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: Mon, 01 Nov 2021 12:19:07 -0000 Hi! On Wed, Sep 01, 2021 at 11:13:43AM -0500, Bill Schmidt wrote: > * config/rs6000/rs6000-call.c (rs6000_invalid_new_builtin): > Implement. That fits on one line. Don't wrap early, esp. not if that leaves a colon without anything following it on that line: it looks like something is missing. > (rs6000_expand_ldst_mask): Likewise. > (rs6000_init_builtins): Initialize altivec_builtin_mask_for_load. > static void > rs6000_invalid_new_builtin (enum rs6000_gen_builtins fncode) > { > + size_t uns_fncode = (size_t) fncode; Like in the previous patch, the "uns_*" name made me think "you do not need an explicit cast, the assignment will do that automatically". But of course it does not matter this is unsigned at all: the cast is casting an enum to a number, which in C++ does require a cast. So maybe you can think of some better name? Something like "j" is fine with me as well btw, it's nice and short, and it is clear you do not want more meaning ;-) > + switch (rs6000_builtin_info_x[uns_fncode].enable) > + case ENB_P6: > + error ("%qs requires the %qs option", name, "-mcpu=power6"); > + break; > + case ENB_CELL: > + error ("%qs is only valid for the cell processor", name); > + break; Maybe "%qs requires the %qs option", name, "-mcpu=cell" ? Boring is good ;-) > + }; (This is switch (...) { ... }; ) Stray semi. Was there no warning? > rtx > rs6000_expand_ldst_mask (rtx target, tree arg0) > { > + int icode2 = BYTES_BIG_ENDIAN You do not need a line break here. > + ? (int) CODE_FOR_altivec_lvsr_direct > + : (int) CODE_FOR_altivec_lvsl_direct; You can align the ? and : just fine without it. > + rtx op, addr, pat; Don't declare such things early. Okay for trunk with those things fixed. Thanks! Segher