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 8187338540BD for ; Mon, 12 Dec 2022 16:22:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8187338540BD 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 2BCGL1bD000434; Mon, 12 Dec 2022 10:21:01 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 2BCGL1D9000430; Mon, 12 Dec 2022 10:21:01 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 12 Dec 2022 10:21:01 -0600 From: Segher Boessenkool To: HAO CHEN GUI Cc: gcc-patches , David , "Kewen.Lin" , Peter Bergner Subject: Re: [PATCH v5, rs6000] Change mode and insn condition for VSX scalar extract/insert instructions Message-ID: <20221212162100.GD25951@gate.crashing.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi! On Fri, Dec 02, 2022 at 03:03:46PM +0800, HAO CHEN GUI wrote: > For scalar extract/insert instructions, exponent field can be stored in a > 32-bit register. So this patch changes the mode of exponent field from DI to > SI so that these instructions can be generated in a 32-bit environment. Also > it removes TARGET_64BIT check for these instructions. > This patch also changes prototypes and catagories of relevant built-ins and > effective target checks of test cases. You can not change the types of builtins, unless they were completely broken before or something serious like that. Existing user code should keep doing the same as it did before. So any change like this needs a lot more justification. > --- a/gcc/config/rs6000/rs6000-builtins.def > +++ b/gcc/config/rs6000/rs6000-builtins.def > @@ -2833,6 +2833,11 @@ > const signed int __builtin_dtstsfi_ov_td (const int<6>, _Decimal128); > TSTSFI_OV_TD dfptstsfi_unordered_td {} > > + const unsigned int __builtin_vsx_scalar_extract_exp (double); > + VSEEDP xsxexpdp {} > + > + const double __builtin_vsx_scalar_insert_exp_dp (double, unsigned int); > + VSIEDPF xsiexpdpf {} vec_insert_exp etc. are externally documented (in the PVIPR), these (internal) builtins should behave similarly. You can change the *internal* stuff fine of course. > --- a/gcc/config/rs6000/rs6000-overload.def > +++ b/gcc/config/rs6000/rs6000-overload.def > @@ -4507,9 +4507,9 @@ > VSESQP > > [VEC_VSIE, scalar_insert_exp, __builtin_vec_scalar_insert_exp] > - double __builtin_vec_scalar_insert_exp (unsigned long long, unsigned long long); > + double __builtin_vec_scalar_insert_exp (unsigned long long, unsigned int); > VSIEDP > - double __builtin_vec_scalar_insert_exp (double, unsigned long long); > + double __builtin_vec_scalar_insert_exp (double, unsigned int); > VSIEDPF NAK on this as well. > ;; VSX Scalar Extract Exponent Double-Precision > (define_insn "xsxexpdp" > - [(set (match_operand:DI 0 "register_operand" "=r") > - (unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")] > + [(set (match_operand:SI 0 "register_operand" "=r") > + (unspec:SI [(match_operand:DF 1 "vsx_register_operand" "wa")] > UNSPEC_VSX_SXEXPDP))] > - "TARGET_P9_VECTOR && TARGET_64BIT" > + "TARGET_P9_VECTOR" > "xsxexpdp %0,%x1" > [(set_attr "type" "integer")]) You need to keep a version that returns DImode (where it did before, 64-bit). But you can use :GPR perhaps? That also allows :SI on 64-bit, so things can be optimised to that where it makes sense. And :SI on 32-bit is just new, no problems there :-) > @@ -5116,7 +5116,7 @@ (define_insn "xsxsigdp" > [(set (match_operand:DI 0 "register_operand" "=r") > (unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")] > UNSPEC_VSX_SXSIG))] > - "TARGET_P9_VECTOR && TARGET_64BIT" > + "TARGET_P9_VECTOR && TARGET_POWERPC64" That is a good fix, separate from the rest though, so could you do it first please? Segher