From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30647 invoked by alias); 9 May 2016 17:36:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 30627 invoked by uid 89); 9 May 2016 17:36:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: e35.co.us.ibm.com Received: from e35.co.us.ibm.com (HELO e35.co.us.ibm.com) (32.97.110.153) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 09 May 2016 17:36:07 +0000 Received: from localhost by e35.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 9 May 2016 11:36:05 -0600 Received: from d03dlp03.boulder.ibm.com (9.17.202.179) by e35.co.us.ibm.com (192.168.1.135) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 9 May 2016 11:35:58 -0600 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: wschmidt@linux.vnet.ibm.com X-IBM-RcptTo: gcc-patches@gcc.gnu.org;segher@kernel.crashing.org Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id D1D1719D804A; Mon, 9 May 2016 11:35:40 -0600 (MDT) Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u49HZvYZ47055098; Mon, 9 May 2016 10:35:57 -0700 Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5AC5078038; Mon, 9 May 2016 11:35:57 -0600 (MDT) Received: from [9.48.101.0] (unknown [9.48.101.0]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP id 11ACC78037; Mon, 9 May 2016 11:35:57 -0600 (MDT) Subject: Re: [PATCH,rs6000] Add built-in support for new Power9 darn (deliver a random number) instruction From: Bill Schmidt To: Segher Boessenkool Cc: Kelvin Nilsen , gcc-patches@gcc.gnu.org In-Reply-To: <20160509135835.GB31139@gate.crashing.org> References: <572B7419.7030205@linux.vnet.ibm.com> <20160509135835.GB31139@gate.crashing.org> Content-Type: text/plain; charset="UTF-8" Date: Mon, 09 May 2016 17:36:00 -0000 Message-ID: <1462815356.6359.1.camel@oc8801110288.ibm.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16050917-0013-0000-0000-00003B6955F6 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg00654.txt.bz2 On Mon, 2016-05-09 at 08:58 -0500, Segher Boessenkool wrote: > Hi Kelvin, > > On Thu, May 05, 2016 at 10:26:01AM -0600, Kelvin Nilsen wrote: > > (UNSPEC_DARN_32): New usnpec constant. > > Typo. > > > ("darn_32"): New instruction. > > We don't normally use quotes for insn names. > > > (rs6000_builtin_mask_calculate): Add in the RS6000_BTM_MODULO and > > RS6000_BTM_64BIT flags to the returned mask, depending on > > configuration. > > Trailing space (many, in this changelog). > > > --- gcc/config/rs6000/altivec.h (revision 235884) > > +++ gcc/config/rs6000/altivec.h (working copy) > > @@ -382,6 +382,11 @@ > > #define vec_vsubuqm __builtin_vec_vsubuqm > > #define vec_vupkhsw __builtin_vec_vupkhsw > > #define vec_vupklsw __builtin_vec_vupklsw > > + > > +/* Non-Vector additions added in ISA 3.0. */ > > +#define darn __builtin_darn > > +#define darn_32 __builtin_darn_32 > > +#define darn_raw __builtin_darn_raw > > #endif > > Do we really want to #define short words like "darn"? If this is already > set in stone, so be it. I don't think we do, and in any case altivec.h would not be the place to do it. darn is not a vector instruction. For these, just having __builtin_darn* be the available interfaces will be fine. My two cents, Bill > > > +(define_insn "darn_32" > > + [(set (match_operand:SI 0 "register_operand" "") > > The constraint should be "r" I suppose? > > > + (unspec:SI [(const_int 0)] UNSPEC_DARN_32))] > > + "TARGET_MODULO" > > + { > > + return "darn %0,0"; > > + } > > + [(set_attr "type" "add") > > Trailing spaces. "add" isn't the correct type; use "integer" if there > is no better type. > > > + (set_attr "length" "4")]) > > That is the default, no need to mention it. Most insns are implicitly > length 4. > > > +/* Miscellaneous builtins for instructions added in ISA 3.0. These > > + instructions don't require either the DFP or VSX options, just the basic > > Trailing space. > > > @@ -3634,6 +3639,8 @@ rs6000_builtin_mask_calculate (void) > > | ((rs6000_cpu == PROCESSOR_CELL) ? RS6000_BTM_CELL : 0) > > | ((TARGET_P8_VECTOR) ? RS6000_BTM_P8_VECTOR : 0) > > | ((TARGET_P9_VECTOR) ? RS6000_BTM_P9_VECTOR : 0) > > + | ((TARGET_MODULO) ? RS6000_BTM_MODULO : 0) > > + | ((TARGET_64BIT) ? RS6000_BTM_64BIT : 0) > > Missing space? > > > + /* RS6000_BTC_SPECIAL represents no-operand operators. */ > > gcc_assert (attr == RS6000_BTC_UNARY > > || attr == RS6000_BTC_BINARY > > - || attr == RS6000_BTC_TERNARY); > > - > > + || attr == RS6000_BTC_TERNARY > > + || attr == RS6000_BTC_SPECIAL); > > + > > Why SPECIAL and not NULLARY or such? > > > + if (rs6000_overloaded_builtin_p (d->code)) > > + { > > + if (! (type = opaque_ftype_opaque)) > > + type = opaque_ftype_opaque > > + = build_function_type_list (opaque_V4SI_type_node, > > + NULL_TREE); > > + } > > Eww. > > if (!opaque_ftype_opaque) > opaque_ftype_opaque = build_function_type_list (...); > type = opaque_ftype_opaque; > > > + enum insn_code icode = d->icode; > > + if (d->name == 0) > > + { > > + if (TARGET_DEBUG_BUILTIN) > > + fprintf (stderr, "rs6000_builtin, bdesc_0arg[%ld] no name\n", > > + (long unsigned)i); > > unsigned is %u, not %d. Space after cast. > > Cheers, > > > Segher >