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 4A774385800A for ; Wed, 18 Nov 2020 12:39:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4A774385800A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=segher@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 0AICcOeO022074; Wed, 18 Nov 2020 06:38:24 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 0AICcM72022073; Wed, 18 Nov 2020 06:38:22 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Wed, 18 Nov 2020 06:38:22 -0600 From: Segher Boessenkool To: Richard Biener Cc: Jeff Law , gcc-patches@gcc.gnu.org, joseph@codesourcery.com, jakub@redhat.com, hp@bitrange.com, will_schmidt@vnet.ibm.com Subject: Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193] Message-ID: <20201118123822.GH2672@gate.crashing.org> References: <20201103231150.zlqccshb3qw63bdv@work-tp> <20201104151049.psotxu7xarcxmv5g@work-tp> <1c09d2bc-da08-523a-709d-ec11140af3f0@redhat.com> 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=-6.4 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 18 Nov 2020 12:39:30 -0000 On Wed, Nov 18, 2020 at 08:31:28AM +0100, Richard Biener wrote: > On Tue, 17 Nov 2020, Jeff Law wrote: > > On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote: > > > On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote: > > >>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the > > >>> + result and setting it in TARGET. Otherwise return NULL_RTX on failure. */ > > >>> +static rtx > > >>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode) > > >>> +{ > > >>> + if (!validate_arglist (exp, VOID_TYPE)) > > >>> + return NULL_RTX; > > >>> + > > >>> + insn_code icode = direct_optab_handler (fegetround_optab, SImode); > > >>> + if (icode == CODE_FOR_nothing) > > >>> + return NULL_RTX; > > >>> + > > >>> + if (target == 0 > > >>> + || GET_MODE (target) != target_mode > > >>> + || !(*insn_data[icode].operand[0].predicate) (target, target_mode)) > > >>> + target = gen_reg_rtx (target_mode); > > >>> + > > >>> + rtx pat = GEN_FCN (icode) (target); > > >>> + if (!pat) > > >>> + return NULL_RTX; > > >>> + emit_insn (pat); > > >> I think you need to verify whether the expansion ended up in 'target' > > >> and otherwise emit a move since usually 'target' is just a hint. > > > I thought the "if (target == 0 ..." took care of that. The expands do > > > emit a move, if that helps. > > It looks like if we have a passed in target and it either has the wrong > > mode or it does not match the predicate, then we generaet a new target > > and use that instead.? I don't see where we'd copy from that new target > > to the original desired target.? For some expanders the caller would > > handle that, but I don't see how that's possible for this one without > > the caller digging into the generated RTL to determine that > > expand_builtin_fegetround put the result somewhere other than TARGET and > > thus a copy is needed. > > > > That may be what Richi is worried about. > > I know we've added missing > > if (!rtx_equal_p (target, ops[0].value)) > emit_move_insn (target, ops[0].value); > > to several expanders (using expand_insn rather than manual > GEN_FCN (icode) calls). We can handle the constants issue similarly to what we do for __builtin_fpclassify, too. Segher